Cherry Pick Workflow To Ensure Consistent Cherry-Picking#438
Cherry Pick Workflow To Ensure Consistent Cherry-Picking#438Flickdm wants to merge 2 commits intomicrosoft:mainfrom
Conversation
This workflow ensures that cherry-picks are consistent and retain the relevant history for integrations.
Flickdm
force-pushed
the
feature/add/cherry-pick-workflow
branch
from
db2f817 to
a15b90e
Compare
.github/workflows/cherry-pick.yml
Outdated
|
|
||
| jobs: | ||
| cherry-pick-job: | ||
| if: contains(github.event.pull_request.title, '[cherry pick]') || contains(github.event.pull_request.title, '[cherry-pick]') |
There was a problem hiding this comment.
This if seems pretty finicky to me. because someone could mess this up in many different ways with capitlization and such.
I see two good ways to solve this:
- Always run this, but have the first step run validation (so maybe lowercase this, remove any "-", "_", or " " in the commit message, and see if [cherrypick] is contained. Produce a variable that is used in the
ifcheck of all other steps - Base this off of a label being added or removed with the trigger
labeled. with theifstatement beingif: github.event.label.name == 'cherry-pick'
With (2) we could also have a check box in the PR for marking something as a cherry-pick. If it is a cherry-pick the auto-labeler will add the cherry-pick label, which would then cause this action to fire.
There was a problem hiding this comment.
Honestly I would like a label but then we would need to identify a thing to be labeled and run into the same problem but I agree this is more brittle than I would like. Maybe we move this logic to the labler and then check for the existence of the label here?
There was a problem hiding this comment.
This
ifseems pretty finicky to me. because someone could mess this up in many different ways with capitlization and such.I see two good ways to solve this:
- Always run this, but have the first step run validation (so maybe lowercase this, remove any "-", "_", or " " in the commit message, and see if [cherrypick] is contained. Produce a variable that is used in the
ifcheck of all other steps- Base this off of a label being added or removed with the trigger
labeled. with theifstatement beingif: github.event.label.name == 'cherry-pick'With (2) we could also have a check box in the PR for marking something as a cherry-pick. If it is a cherry-pick the auto-labeler will add the cherry-pick label, which would then cause this action to fire.
I see you edited your comment - a new box for a "cherry-pick" could be a good idea
There was a problem hiding this comment.
As You commented on Olivers message, you confirmed contains is not case sensitive, so this is less of a concern for me. I would maybe just add cherry_pick as a possible option, worst case.
There was a problem hiding this comment.
If we want this, I suggest the checkbox + label. Then, it is a straightforward opt-in with the label attached for further filtering versus the opportunity for typos and/or misunderstanding what the tag in the PR title means.
There was a problem hiding this comment.
Can this just be added to Pull Request Formatting Validator?
There was a problem hiding this comment.
Do you think it makes sense to add this to the general pull request formatter? I'm happy to add it there if you think it's worth it. I was just trying to create it as a separate workflow to separate the logic
There was a problem hiding this comment.
That's meant to be a collection of checks for formatting. You can still conditionalize this on a "cherry-pick trigger".
.github/workflows/cherry-pick.yml
Outdated
| continue | ||
| fi | ||
|
|
||
| if [[ ! $(echo "$commit_message" | tr '[:upper:]' '[:lower:]') =~ \[cherry-pick\/.+\] ]] || [[ ! "$commit_message" =~ \(cherry\ picked\ from\ commit\ [0-9a-f]{40}\) ]]; then |
There was a problem hiding this comment.
The user visible messages are going to need to be more visible in the PR (comments). Many people don't know to look at actions log messages.
|
Closing for now. @Flickdm agrees. |
4 participants
Description
This workflow ensures that cherry-picks are consistent and retain the relevant history for integrations.
Any PR with [Cherry Pick] or [Cherry-Pick] will trigger this workflow
It will check the following:
If it's a "Merge" commit the commit will be ignored. (Needed during PR)
If the commit violates one of these policies, then the PR gate will fail with the failing commit
How This Was Tested
Flickdm/mu_tiano_plus#1
Integration Instructions
N/A