Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions .github/workflows/cherry-pick.yml
Copy link
Member

Choose a reason for hiding this comment

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

Can this just be added to Pull Request Formatting Validator?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

That's meant to be a collection of checks for formatting. You can still conditionalize this on a "cherry-pick trigger".

Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# This workflow validates commit messages in a pull request to ensure they follow a specific pattern.
# The pattern is defined as follows:
# - The commit message must contain the string "[cherry-pick/XXX]" where XXX is a valid label.
# - The commit message must also contain "(cherry picked from commit <commit_hash>)" where <commit_hash> is a 40-character SHA-1 hash.
# - Merge commits are ignored.
#
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent

name: Cherry-Pick Workflow

on:
pull_request:
types:
- edited
- opened
- reopened
- synchronize
workflow_dispatch:

jobs:
cherry-pick-job:
if: contains(github.event.pull_request.title, '[cherry pick]') || contains(github.event.pull_request.title, '[cherry-pick]')
Copy link
Contributor

@Javagedes Javagedes May 5, 2025

Choose a reason for hiding this comment

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

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:

  1. 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 if check of all other steps
  2. Base this off of a label being added or removed with the trigger labeled. with the if statement being if: 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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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:

  1. 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 if check of all other steps
  2. Base this off of a label being added or removed with the trigger labeled. with the if statement being if: 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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#contains

Copy link
Member

Choose a reason for hiding this comment

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

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.

runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: actions/checkout@v2
with:
fetch-depth: 0 # Fetch the full history to ensure all commits are available

- name: Validate commit messages
run: |
base_branch="${{ github.event.pull_request.base.ref }}"

# Ensure the base branch is fetched
git fetch origin $base_branch

# Get the commits included in the PR
commits=$(git log --pretty=format:%H origin/$base_branch..HEAD)

echo "Number of commits being reviewed: $(echo "$commits" | wc -w)"

for commit in $commits; do
commit_message=$(git log --format=%B -n 1 "$commit")

if [[ "$commit_message" =~ ^Merge ]]; then
echo "Warning: Commit $commit is a merge commit and will be ignored."
continue
fi

if [[ ! $(echo "$commit_message" | tr '[:upper:]' '[:lower:]') =~ \[cherry-pick\/.+\] ]] || [[ ! "$commit_message" =~ \(cherry\ picked\ from\ commit\ [0-9a-f]{40}\) ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

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.

echo "Commit $commit does not meet the required pattern."
echo "Commit message: $commit_message"
echo "Please use 'git cherry-pick -x <commit_hash>' to cherry-pick the commit."
exit 1
else
echo "Commit $commit meets the required pattern."
fi
done

echo "All commit messages meet the required pattern.";