Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove text on GITHUB_TOKEN which is now built-in #8907

Merged
merged 2 commits into from Dec 16, 2021

Conversation

letmaik
Copy link
Contributor

@letmaik letmaik commented Dec 16, 2021

Just a documentation update.

@ashmaroli
Copy link
Member

I agree with your take that there's no explicit need to create GITHUB_TOKEN. But creating a PAT grants freedom on the scope.
I'll approve this PR if you will add a link to the documentation on the restricted scope-flexibility of GITHUB_TOKEN and consequently recommend using a PAT for additional flexibility.

@letmaik
Copy link
Contributor Author

letmaik commented Dec 16, 2021

Could you go into more detail on how a PAT allows to limit the scope further? GITHUB_TOKEN is scoped to the current repo and can be further limited through

    permissions:
      contents: write

A PAT cannot be scoped to a single repo. The thing I don't fully understand is how the public_repo OAuth scope compares to the contents: write scope of GITHUB_TOKEN.

@ashmaroli
Copy link
Member

Could you go into more detail on how a PAT allows to limit the scope further?

I think you misread my comment. I was referring to the limited scope of the auto-generated GITHUB_TOKEN w.r.t to a PAT currently recommended by the live Jekyll Documentation..

Anyways, the GitHub docs on the relevant info are at:

@letmaik
Copy link
Contributor Author

letmaik commented Dec 16, 2021

Could you go into more detail on how a PAT allows to limit the scope further?

I think you misread my comment. I was referring to the limited scope of the auto-generated GITHUB_TOKEN w.r.t to a PAT currently recommended by the live Jekyll Documentation..

I was caught up by "consequently recommend using a PAT for additional flexibility". I can't think of any scenario where it makes sense to recommend using a PAT for github pages.

@ashmaroli
Copy link
Member

I can't think of any scenario where it makes sense to recommend using a PAT for github pages.

Fair enough.
I'll merge when the CI run finishes.
Thanks @letmaik

@ashmaroli
Copy link
Member

@jekyllbot: merge +doc

@jekyllbot jekyllbot merged commit 23af360 into jekyll:master Dec 16, 2021
jekyllbot added a commit that referenced this pull request Dec 16, 2021
github-actions bot pushed a commit that referenced this pull request Dec 16, 2021
Maik Riechert: Remove text on GITHUB_TOKEN which is now built-in (#8907)

Merge pull request 8907
@letmaik letmaik deleted the patch-1 branch December 16, 2021 22:17
@jekyll jekyll locked and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants