-
Notifications
You must be signed in to change notification settings - Fork 4.6k
PM: Add checks for author association in first-time contributor label… #73965
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
base: trunk
Are you sure you want to change the base?
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
cc @talldan |
|
It looks like the core repo is using |
packages/project-management-automation/lib/tasks/first-time-contributor-label/index.js
Outdated
Show resolved
Hide resolved
9b65bf2 to
54cb6d8
Compare
| `first-time-contributor: Author association is ${ authorAssociation }. Aborting` | ||
| ); | ||
| return; | ||
| } |
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.
With this in place, do we still need the older check based on octokit.rest.repos.listCommits?
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.
I've left it for legacy purposes or failover... but technically not.
| debug( | ||
| `first-time-contributor: Searching for commits in ${ owner }/${ repo } by @${ author }` | ||
| ); | ||
|
|
||
| const { data: commits } = await octokit.rest.repos.listCommits( { | ||
| owner, | ||
| repo, | ||
| author, | ||
| } ); | ||
|
|
||
| if ( commits.length > 0 ) { | ||
| debug( | ||
| `first-time-contributor-label: Not the first commit for author. Aborting` | ||
| ); | ||
| return; | ||
| } |
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.
@mcsf your idea is removing this block?
I just wanted to mention that I don't love having to rely on this third-party action for a simple comment. Especially with the |
Following the troubles with First Time Contribution triggering wrongly
Like in:
#73961 (comment).
I'm proposing here adding the
author_associationto check for more trustworthy references.More info: https://michaelheap.com/github-actions-check-permission/