Improve concurrency settings for PR checks
#3405
Conversation
Since `github.workflow` will be the caller's name for `workflow_call` events
There was a problem hiding this comment.
Pull request overview
This PR improves the concurrency settings for PR check workflows by including workflow input parameters in the concurrency group name. This allows workflows with different input combinations to run in parallel while ensuring that workflows with the same inputs and ref are properly grouped for cancellation or queuing.
Changes:
- Modified the Python generator script to append all workflow inputs to the concurrency group name
- Updated the
cancel-in-progressexpression (added redundant|| falseclause) - Regenerated all 61 workflow files with the new concurrency group naming pattern
Reviewed changes
Copilot reviewed 56 out of 56 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pr-checks/sync.py | Updated concurrency group generation to include workflow input values; modified cancel-in-progress expression |
| .github/workflows/__*.yml (61 files) | Auto-generated workflow files reflecting the new concurrency group naming pattern with input values |
| 'cancel-in-progress': "${{ github.event_name == 'pull_request' || false }}", | ||
| # The group is determined by the workflow name + the ref | ||
| 'group': "${{ github.workflow }}-${{ github.ref }}" | ||
| 'group': checkName + "-${{github.ref}}" + extraGroupName |
There was a problem hiding this comment.
The YAML dumper is wrapping long 'group' values onto the next line (e.g., see __with-checkout-path.yml lines 62-63), while shorter values remain on the same line (e.g., __swift-autobuild.yml line 32). While this may technically be valid YAML (as a plain scalar in block context), it's unconventional and inconsistent. Consider configuring the YAML dumper to prevent line wrapping for better readability and consistency, either by setting a larger width (e.g., yaml.width = 200) or by using a string wrapper class that forces inline formatting.
| # consequently the number of concurrent API requests. | ||
| 'cancel-in-progress': "${{ github.event_name == 'pull_request' }}", | ||
| 'cancel-in-progress': "${{ github.event_name == 'pull_request' || false }}", | ||
| # The group is determined by the workflow name + the ref |
There was a problem hiding this comment.
The comment states "The group is determined by the workflow name + the ref" but this is now outdated. The group also includes workflow input values. Consider updating the comment to reflect this, such as: "The group is determined by the workflow name + the ref + the input values".
| # This should help reduce the number of concurrent workflows on the repo, and | ||
| # consequently the number of concurrent API requests. | ||
| 'cancel-in-progress': "${{ github.event_name == 'pull_request' }}", | ||
| 'cancel-in-progress': "${{ github.event_name == 'pull_request' || false }}", |
There was a problem hiding this comment.
The expression || false is redundant in the cancel-in-progress setting. The comparison github.event_name == 'pull_request' already returns a boolean value (true or false), so adding || false doesn't change the behavior. Consider simplifying to just ${{ github.event_name == 'pull_request' }}.
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
How did/will you validate this change?
pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist