-
Notifications
You must be signed in to change notification settings - Fork 805
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
Gardening action: add new Flag OSS feature #19382
Conversation
- I chose to only add a label for now. In the future we could opt to do more (add a welcome comment, ...) - I chose to look at how they worked on their PR (from a fork) instead of looking at the organization listed in their profile to decide whether they would be considered as outside contributors.
This brings the feature in line with the existing tool we use today in the Calypso repo.
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. |
We have fork checks inside the action anyway.
That's not an option from forks. We could aim to open things up via pull_request_target. Not doing that just yet though. See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
I'll close this for now. The limitations of |
Since those events will need to be triggered for folks using forks, we cannot rely on just pull_request.
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.
Let's try it. I'll leave it to you to merge @jeherve so you can test with a fork post-merge.
Follow-up from #19382. We need to allow the action to run from forks for pull_request_target events, so we can add labels to PRs created by external contributors.
Tested this, and it turns out it will need a follow-up: #19554 |
Follow-up from #19382. We need to allow the action to run from forks for pull_request_target events, so we can add labels to PRs created by external contributors.
This allows adding a label and posting a Slack message every time someone opens a PR from a fork. See Automattic/jetpack#19382 Internal reference: p3btAN-1nR-p2#comment-13830
This allows adding a label and posting a Slack message every time someone opens a PR from a fork. See Automattic/jetpack#19382 Internal reference: p3btAN-1nR-p2#comment-13830
This allows adding a label and posting a Slack message every time someone opens a PR from a fork. See Automattic/jetpack#19382 Internal reference: p3btAN-1nR-p2#comment-13830
This allows adding a label and posting a Slack message every time someone opens a PR from a fork. See Automattic/jetpack#19382 Internal reference: p3btAN-1nR-p2#comment-13830
This allows adding a label and posting a Slack message every time someone opens a PR from a fork. See Automattic/jetpack#19382 Internal reference: p3btAN-1nR-p2#comment-13830
Changes proposed in this Pull Request:
This PR brings an additional task to the Gardening action, based on an existing feature currently enabled for the Calypso repo:
This new action flags PRs opened by external contributors (not from a branch on the main repo), adds the "OSS Citizen" label, and posts a Slack message to your team channel to let everyone know you've got a contribution from an external contributor.
Note: I chose to look at how they worked on their PR (from a fork) instead o
f looking at the organization listed in their profile to decide whether they
would be considered as outside contributors.
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
pull_request_target
, we will not be able to test much before this gets merged. Then once this is merged, I'll open a test PR from a fork to test things in the #jeherve-debug channel.