feat: added permissions (pull-requests: read) setting to workflow and README example

[natterstefan]

Changes

I see more and more actions taking care of managing the permissions of the GITHUB_TOKEN, which is why I added it to your example and the workflows (+ updated them to the latest version).

The ones I added to release, and versioning might need some changes once you merge them. I'm pretty sure that's all they need. As your action is widely used and growing, I think it's a good practice to help others get to know the permissions setting and take care of it in their workflows.

Demo PR with Test Runs

• feat: added permissions (pull-requests: read) setting to workflow and README example natterstefan/action-semantic-pull-request#1

Docs

• Assigning permissions to jobs - GitHub Docs
• GitHub Actions: Control permissions for GITHUB_TOKEN | GitHub Changelog
• Workflow syntax for GitHub Actions - GitHub Docs

[amannn]

Hey @natterstefan,

it's nice to see you here! 🙂 Thank you for your PR!

There was some discussion on this in #182 (likely also #212 intended to be about this). I was previously a bit opposed to this since it requires more configuration, but maybe it can in fact be helpful for trading that with trustworthiness.

Do you have examples of other actions which advertise the usage of permissions?

Some considerations (unfortunately GitHub freezes today for me when I open the files tab, so I'll list the feedback here):

• When the wip feature is used, we also require to update statuses. Can you add information about this in the comment for the wip option? I think generally suggesting read permissions for PRs is a good idea.
• Can you please also link to a test run as it's mentioned in the contributors guide? E.g. I'd be curious if using the checkout action only requires the pull-requests: read option, or if a permission like contents is needed. Why does the test workflow use contents btw.?
• I just saw that lint-pr-title-preview-outputErrorMessage mistakenly doesn't use the .yml extension. Do you think you could fix that while you're at it?

Thank you also for upgrading some action versions!

Many thanks! 🙌

[stevenh]

Good to see the addition of permissions, gives users a better understanding of what it needs to do the job. Would love to see this merged.

[natterstefan]

Hi @stevenh, thanks for the reminder. I keep the tab open now, and I hope to get to this in the next few days.

[natterstefan]

Hi @amannn,

• When the wip feature is used, we also require to update statuses. Can you add information about this in the comment for the wip option? I think generally suggesting read permissions for PRs is a good idea.

I am not 100% sure which information I should add to the WIP part of the config in README.md. Can you explain this further to me, please?

• Can you please also link to a test run as it's mentioned in the contributors guide? E.g. I'd be curious if using the checkout action only requires the pull-requests: read option, or if a permission like contents is needed.

I already linked to a Demo PR with test runs in the PR description.

Why does the test workflow use contents btw.?

I am not 100% sure why I've added contents: none in the past, but I have now changed it to contents: read as this is the only permission this workflow requires.

[amannn]

I am not 100% sure which information I should add to the WIP part of the config in README.md. Can you explain this further to me, please?

The built-in support for [WIP] relies on having write permissions to change the status of a PR. If our example configuration advocates for pull-requests: read, users of the wip option likely have to add another permission—this should be documented.

Does this help?

[natterstefan]

The built-in support for [WIP] relies on having write permissions to change the status of a PR. If our example configuration advocates for pull-requests: read, users of the wip option likely have to add another permission—this should be documented.

Does this help?

I got it. It makes sense, and thanks for the info. You see what I came up with for the README.me here: d1c5b62 (#215).

Adding a lint-pr-title-preview-wip.yml action would be great to test in an example PR (or commit). That allows you to scan the example PR for at least one commit that "tests" the feature. But not sure if that's feasible to ask contributors to add [WIP] first and then remove it again after the action has been completed (so you can see it's green) and only then request a review from your side (as they most likely remove it again to proof the other actions work).

[amannn]

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 the wip 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! 🙏

[natterstefan]

Hey @amannn,

please review again and take a look at:

• 1ebd749 (#215)
• f030897 (#215)

[amannn]

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.

[amannn]

Thank you so much @natterstefan! 👏

[natterstefan]

You're welcome @amannn! 🤜🤛

[github-actions]

🎉 This PR is included in version 5.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀