Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: added permissions (pull-requests: read) setting to workflow and README example #215
feat: added permissions (pull-requests: read) setting to workflow and README example #215
Changes from 7 commits
78bfa11
aac1bdb
605dc50
8f18c35
3cdd34e
b7612cc
d1c5b62
1ebd749
f030897
7c7ce76
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good!
Maybe we can move the whole config option for
wip
to this block, similar to what we've done with the legacy configuration below? I'm just worried that users stop reading at thewip
option above and don't notice that permissions need to be changed (even though it's the very next headline).By doing this, we can make sure we're educating users about the required permissions as we're introducing the feature. Maybe you can reuse the description from above here and adjust the headline accordingly to match the whole section?
I just noticed the "Note that a second check will be reported if this is enabled." part is no longer valid, can you remove this as part of moving the text?
Thank you so much! 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @amannn,
please review again and take a look at:
1ebd749
(#215)f030897
(#215)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks! I've included a commit with a few further README improvements that I had on my list to include them in the next release.