8000 Remove text on GITHUB_TOKEN which is now built-in by letmaik · Pull Request #8907 · jekyll/jekyll · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 16, 2021
Merged

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

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
@jekyllbot
Update history to reflect merge of #8907 [ci skip]
2abf178
github-actions bot pushed a commit that referenced this pull request Dec 16, 2021
@jekyllbot
Deploy docs from 23af360
7bc5835 
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
Rockyspade referenced this pull request Aug 12, 2024
@koic
Make custom cop inherit RuboCop::Cop::Base (#9597)
28e10da 
Merge pull request 9597
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ashmaroli ashmaroli ashmaroli approved these changes

Assignees
No one assigned
Labels
documentation frozen-due-to-age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@letmaik @ashmaroli @jekyllbot
0