A Ruleset with Metadata Restrictions on Commit Messages should not block Squash Merge Pull Requests unless the sqash commit message itself violates the rule

[justinmchase]

Select Topic Area

Bug

Body

I have a Ruleset which puts a regex restriction on commit messages.

I have a commit in my pull request which violates the Ruleset rule with Metadata Restrictions on commit messages.

The rule only applies to the main branch and I am making a PR from a feature branch to the main branch.

I would expect this to still allow sqaush merging but the option is disabled.

When doing a squash merge you are presented with an editor which will prepend the PR title and allow you to edit the final commit message which will be the actual commit message that is merged to the protected branch.

I would expect the metadata rule to apply to the final commit message only and not block squash merging unless the rule fails on that message, not each individual commit.

[neoeinstein]

This is definitely a thing that I'd like to have enforced as expected. In the interim, I can work around this to update the ruleset to apply to all branches, but for with a squash merge, only that commit message should matter. Getting this to work as expected would reduce the burden on a developer to use conventional commits before pushing to a PR branch, and, if forgotten, they wouldn't need to go back and rewrite their commits.

[github-actions[bot]]

🕒 Discussion Activity Reminder 🕒

This Discussion has been labeled as dormant by an automated system for having no activity in the last 60 days. Please consider one the following actions:

1️⃣ Close as Out of Date: If the topic is no longer relevant, close the Discussion as out of date at the bottom of the page.

2️⃣ Provide More Information: Share additional details or context — or let the community know if you've found a solution on your own.

3️⃣ Mark a Reply as Answer: If your question has been answered by a reply, mark the most helpful reply as the solution.

Note: This dormant notification will only apply to Discussions with the Question label. To learn more, see our recent announcement.

Thank you for helping bring this Discussion to a resolution! 💬

[IceFlock]

Just ran into this and have had to abandon the idea as a result. I would also expect only the squashed commit to be subject to this rule as its the thing that actually gets put on the target branch,

[github-actions[bot]]

🕒 Discussion Activity Reminder 🕒

This Discussion has been labeled as dormant by an automated system for having no activity in the last 60 days. Please consider one the following actions:

1️⃣ Close as Out of Date: If the topic is no longer relevant, close the Discussion as out of date at the bottom of the page.

2️⃣ Provide More Information: Share additional details or context — or let the community know if you've found a solution on your own.

3️⃣ Mark a Reply as Answer: If your question has been answered by a reply, mark the most helpful reply as the solution.

Note: This dormant notification will only apply to Discussions with the Question label. To learn more, see our recent announcement.

Thank you for helping bring this Discussion to a resolution! 💬

[Ninjaskurk]

Is there any workaround for this?

For squash and merge the individual commits wont be visible in the main branch, so we don't actually care about those with this type of protection. We only care about the title of the PR, and if that follows the rule then squash and merge should be possible.

[justinmchase]

The workaround is to not use a Rulset for this but to just create an action which does a regular expression check right in the action.

[justinmchase]

Yeah I just made an action in a repo, for example like this:

actions/conventional-commits/action.yml

name: Conventional Commits
description: Check that a given string matches the conventional commits format
inputs:
  title:
    type: string
    required: true

runs:
  using: node20
  main: index.js

actions/conventional-commits/index.js

const title = process.env[`INPUT_TITLE`]
if (!/^(build|chore|ci|docs|feat|fix|perf|refactor|revert|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)/.test(title)) {
  console.log("::error title=PR Title::Your pull request title does not follow the conventional commit format. Please use the format: <type>(<scope>): <description>")
  console.log("::warning title=Available Types::build,chore,ci,docs,feat,fix,perf,refactor,revert,test");
  console.log("::warning title=Example::chore(dependency): update nuget dependencies");
  process.exit(1)
}

.github/workflows/check.yml

name: Check
on:
  pull_request_target:
    types: [edited, opened, reopened, synchronize]

jobs:
  pr_title:
    name: Pull Request Title
    runs-on: ubuntu-latest
    steps:
    - name: Conventional Commits
      if: github.event.pull_request != null
      uses: example/shared/actions/conventional-commits@v1
      with:
        title: ${{ github.event.pull_request.title }}

Then in one of your Rulesets you add a requirement on pull requests to have Pull Request Title workflow.

[justinmchase]

What I can't really figure out is when you run this action. If you run it in the pull request, nothing prevents the user from changing the message when doing the squash, and if you run it in the main branch, it's already too late. Am I missing something?

Ah I see, yeah the trick is to use the event pull_request_target. This will run the workflow code that is in the main branch, so even if they change it in their PR it will not take effect until merged into main.

The only trouble with it is if you're changing the action or workflow itself it will be a little confusing, to solve that you can add a workflow_dispatch and just manually invoke it to prove its working as desired.

[justinmchase]

I submitted a Support Ticket for this finally, as I believe it is genuinely a bug.

[stefan-olli]

Did you get any response to the support ticket?

[gshaibi]

Yeah I just made an action in a repo, for example like this:

actions/conventional-commits/action.yml

name: Conventional Commits
description: Check that a given string matches the conventional commits format
inputs:
  title:
    type: string
    required: true

runs:
  using: node20
  main: index.js

actions/conventional-commits/index.js

const title = process.env[`INPUT_TITLE`]
if (!/^(build|chore|ci|docs|feat|fix|perf|refactor|revert|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)/.test(title)) {
  console.log("::error title=PR Title::Your pull request title does not follow the conventional commit format. Please use the format: <type>(<scope>): <description>")
  console.log("::warning title=Available Types::build,chore,ci,docs,feat,fix,perf,refactor,revert,test");
  console.log("::warning title=Example::chore(dependency): update nuget dependencies");
  process.exit(1)
}

.github/workflows/check.yml

name: Check
on:
  pull_request_target:
    types: [edited, opened, reopened, synchronize]

jobs:
  pr_title:
    name: Pull Request Title
    runs-on: ubuntu-latest
    steps:
    - name: Conventional Commits
      if: github.event.pull_request != null
      uses: example/shared/actions/conventional-commits@v1
      with:
        title: ${{ github.event.pull_request.title }}

Then in one of your Rulesets you add a requirement on pull requests to have Pull Request Title workflow.

This will only make validation on the PR title, the commit title may be different, right? (even though the PR title is the default squashed commit title)

[Usaldudo]

This issue has impacted our team as well. We support this suggestion to only block on the final squash commit message.

[FearKamikadze]

We would benefit from this change too. The ruleset should not block squash merges based on individual commit messages.

[leros1337]

Workaround using ci is really bad and overkill on lot activity repos, need some trigger in rulesets about squash merges

[AbdelrhmanHamouda]

i don't know why such a popular request (not the first time it appears) is being ignored this much!

[dexterorion]

another one here

followed @justinmchase solution for now, but it would be great to handle this using ruleset

[clausjoergensen]

Whelp, this is a shame. It makes the commit rules completely unusable for us. I guess we'll continue with our GitHub Action to validate the PR title instead.

Shame the GitHub developers didn't consider something as simple as a squash merge. Who on earth would want to enforce such conventions on every single commit in a PR? It defeats the whole purpose of a PR! Might as well have everyone committing directly to master if not.

[github-actions[bot]]

🕒 Discussion Activity Reminder 🕒

This Discussion has been labeled as dormant by an automated system for having no activity in the last 60 days. Please consider one the following actions:

1️⃣ Close as Out of Date: If the topic is no longer relevant, close the Discussion as out of date at the bottom of the page.

2️⃣ Provide More Information: Share additional details or context — or let the community know if you've found a solution on your own.

3️⃣ Mark a Reply as Answer: If your question has been answered by a reply, mark the most helpful reply as the solution.

Note: This dormant notification will only apply to Discussions with the Question label. To learn more, see our recent announcement.

Thank you for helping bring this Discussion to a resolution! 💬

[justinmchase]

This is still valid, we're still waiting on someone from github to fix it.

[devinnived]

It sucks that the commit message restriction requires an enterprise plan but we don't get enterprise support 👎🏼

[kakid]

Hello,
Are there any planned improvements for this topic?
Thank you :)

[Sjnahak]

Hi @justinmchase
Hope you are doing good , did you get any response on the support ticket for this issue?

[justinmchase]

nothing yet, it appears to not be on GitHub radar.

[SvenStaehs]

what a shame, I also just ran into this (and this is the first google result one gets). The marked answer really does not solve the underlying problem.

The PR title is part of the review, so if it's wrong the reviewer is expected to flag that. What we really want to ensure is the commits on release branches so they can be parsed for semantic releases. Some implementations (python-semantic-release) fail when they encounter a non-conforming commit, others ignore it. Luckily we just switched to one that ignores it, but it's still an issue for release note creation!

I guess we can have a PR check for title conformity, plus a push-triggered workflow to verify after the fact and flag it for a maintainer to consider whether it's egregious enough to warrant changing the history of main, or manually fix the changelog.