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
Update concurrency group for community label workflow #47355
Update concurrency group for community label workflow #47355
Conversation
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
…x-concurrency-group-pull_request_target
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 just had the one question about simplifying the change.
@@ -7,7 +7,9 @@ on: | |||
types: [opened] | |||
|
|||
concurrency: | |||
group: ${{ github.workflow }}-${{ github.ref }} |
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.
@adimoldovan Could github.ref
be replaced with github.head_ref
to get the PR branch instead of trunk?
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.
It would help if we only had the pull_request_target
event, but we also have the issues
event for which github.head_ref
is undefined.
On the other hand the group could be simpler in our case, because we only have the opened
action for both events. I went with a group that would cover more actions in case they would be added later.
I guess we could go with the shorter community-label-${{ github.event_name == 'pull_request_target' && github.head_ref || github.run_id }}
.
What do you think?
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.
That makes sense. Looks good as is.
Hi @rrennick, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
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 good.
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #47179
Updated the concurrency group for the
community-label.yml
workflow to use an unique group. The github.ref is always returningrefs/heads/trunk
incorrectly cancelling some runs.I also checked for other workflows using
issues
orpull_request_target
triggers but no other was using concurrency.How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
After merge, open two issues at the same time. The
Add Community Label, Assign Reviewers
should run for both, the first run should not be cancelled by the second one.Repeat the same test with two pull requests.