search

blahgeek / emacs-pr-review

GNU General Public License v3.0
43 stars 2 forks source link

Support fine-grained tokens #4

Open junyeong-huray opened 1 year ago

junyeong-huray commented 1 year ago

Hello

Thank you for writing great package! I am so delightful to find this.

I found M-x pr-review works as expected with classic PAT but it raises an error when I put the Fined-grained tokens (Beta) into ~/.authinfo.

This message is printed at the *Messages* buffer.

pr-review--execute-graphql: Error while making graphql request get-pull-request: FORBIDDEN: Resource not accessible by personal access token

I granted all repository permissions, and I selected correct repositories in the PAT setting page.

Can you check that pr-review is able to work with the Fine grained token (Beta)?

Thanks,

blahgeek commented 1 year ago

It seems to be a limitation on github side:

https://github.blog/2022-10-18-introducing-fine-grained-personal-access-tokens-for-github/

We also have a set of enhancements planned that we intend to address before making fine-grained personal access tokens generally available. These changes include:

  • Support for GraphQL. Currently, fine-grained PATs can only be used against the REST APIs

This package makes heavy use of GraphQL. So I guess all we can do is wait for github's support.

junyeong-huray commented 1 year ago

Thank you for letting me know this. I see that it's a limitation originated from github.

jarodevs commented 1 month ago

Would it be possible to specify what are the scopes needed for the classic access token? Right now I have it setup with everything related to repo selected.

I have not experimented with the package enough to know if I will run into issues with that configuration. Would be nice to have that added to the README

blahgeek commented 1 month ago

Would it be possible to specify what are the scopes needed for the classic access token? Right now I have it setup with everything related to repo selected.

I have not experimented with the package enough to know if I will run into issues with that configuration. Would be nice to have that added to the README

I don't know it for sure, as the github documentation does not seem to specify required scopes for each graphql APIs. Based on my personal usage,enabling "repo" and "notification" should be sufficient, though it may not be the minimal required set. To find out the minimal required set, I recommend you start at minimum and try for yourself. Insufficient scopes would lead to error during usage. I just pushed a new commit that improved the error messages in such scenario.