Skip to content
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

meta: post comment when pr labeled fast-track #38446

Closed
wants to merge 2 commits into from
Closed

meta: post comment when pr labeled fast-track #38446

wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 28, 2021

Automatically adds a comment when a pull requests is labeled "fast-track"

Signed-off-by: James M Snell [email protected]

@github-actions github-actions bot added the meta Issues and PRs related to the general management of the project. label Apr 28, 2021
@jasnell jasnell added fast-track PRs that do not need to wait for 48 hours to land. and removed fast-track PRs that do not need to wait for 48 hours to land. labels Apr 28, 2021
@jasnell
Copy link
Member Author

jasnell commented Apr 28, 2021

@nodejs/build ... my github actions skill is still fairly weak so I'd appreciate if someone could help verify this.

@targos
Copy link
Member

targos commented Apr 28, 2021 •
edited
Loading

Unfortunately you cannot test changes to a workflow that uses the pull_request_target event without landing them on a branch and opening a PR against that branch

@aduh95
Copy link
Contributor

aduh95 commented Apr 28, 2021

Maybe this should be merged with .github/workflows/comment-stalled.yml and called something like auto-comments.yml? I suppose we may want to add more automation in the future, adding a new workflow each time seems unnecessary.

@jasnell
Copy link
Member Author

jasnell commented Apr 28, 2021

@targos @aduh95 ... updated! I combined the label actions into a single workflow and I'm using the gh cli for the fast track action. The gh cli could also be used for the stalled action but I'd prefer to do that in a separate PR.

targos
targos approved these changes Apr 28, 2021
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The file could be renamed to "comment-labeled.yml" or "labeled.yml"

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 28, 2021
mmarchini
mmarchini approved these changes Apr 29, 2021
View reviewed changes
Copy link
Contributor

@mmarchini mmarchini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would prefer the files to remain separate, merging them doesn't bring any value because there's no reuse between the two flows, and the only thing they share is the labeled event for PRs. Plus merging the files increase the complexity because now we need an extra condition in the if statement to skip the fast track job on issues.

I don't feel strong enough about it to ask to go back to separate files though.

@jasnell
Copy link
Member Author

jasnell commented Apr 30, 2021 •
edited
Loading

@mmarchini ... I don't know if I was doing something wrong or what, but I actually had a difficult time getting the fastTrack workflow to run consistently when it was in a separate file.... and by that I mean in testing on my personal fork, the stalled label workflow would run but the fast track wouldn't. The only way I was able to get both to run consistently was putting them both in a single workflow file. Not opposed to separating them back out into separate files if that works and is preferred but let's go ahead and land this PR and we can separate them out later if necessary in another commit.

@jasnell
Copy link
Member Author

jasnell commented Apr 30, 2021

Landed in d85929c

@jasnell jasnell closed this Apr 30, 2021
jasnell added a commit that referenced this pull request Apr 30, 2021
@jasnell
meta: post comment when pr labeled fast-track
d85929c 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #38446
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request May 3, 2021
@jasnell @targos
meta: post comment when pr labeled fast-track
96f54d3 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #38446
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@targos targos mentioned this pull request May 3, 2021
Merged
targos pushed a commit that referenced this pull request May 30, 2021
@jasnell @targos
meta: post comment when pr labeled fast-track
df1b29c 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #38446
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
@jasnell @targos
meta: post comment when pr labeled fast-track
a6d4dcd 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #38446
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
@jasnell @targos
meta: post comment when pr labeled fast-track
9df3fc5 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #38446
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@targos targos mentioned this pull request Jun 6, 2021
Merged
targos pushed a commit that referenced this pull request Jun 11, 2021
@jasnell @targos
meta: post comment when pr labeled fast-track
fcc6a00 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #38446
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivikr trivikr trivikr approved these changes

@richardlau richardlau richardlau approved these changes

@mmarchini mmarchini mmarchini approved these changes

@targos targos targos approved these changes

@antsmartian antsmartian antsmartian approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jasnell @targos @aduh95 @antsmartian @mmarchini @richardlau @trivikr