Meta data ruleset regex on commit messages regex failing incorrectly

[Willshaw]

Select Topic Area

Bug

Body

I'm using the Beta rulesets in Evaluate mode to see how they'll affect our repo.

Using this regex ((Merge )|(\[ABC-[0-9]+\])).+ gives me back the following in regexpal.com:

# pass
[ABC-2794]foo bar
[ABC-2794] foo bar
[ABC-2794]: foo bar
Merge pull request #1234 foo bar
Merge branch foo bar

# fail
ABC-1234 foo bar
ABC-1234
foo bar
Merging foo bar
Merging branch foo bar

So I'm happy with what regexpal is telling me, but my insights are only passing the [SCH]... PRs and failing on any that start Merge pull and Merge branch

I'd rather use simpler regex, but I can't see a mechanism of adding multiple rules (as in commits must pass one or the other)

Is there something specific about the ruleset/Github regex engine I've missed?

[patrick-knight]

Hey @Willshaw sorry for the way delayed response here.

1. Is this behavior still occurring for you? We shipped a number of changes since your post. (again thanks for the patience)
2. Can you drop a screenshot of the rule and insight, if this still an issue?

Thanks!

[Willshaw]

I've lost admin rights to the repo and we've since found a workaround using Github Actions now instead.

[Mihai-IDP]

It might not be the same problem as OPs (but I'm quite confident that this is the cause), but here is why merge pushes failed the commit message rule for us (both from local merges and from a PR in Github) even though the rule's regex accounted for the "Merge" keyword, besides the task ID and other keywords (very similar to OPs regex).

The regex itself and how Github tests for it is not the issue.

The issue is that we implemented the regex rule only recently. We have old commits on different branches with messages that wouldn't pass the current rule. When you merge one of the older branches (PR or without), that push of the merge doesn't only contain the "Merge branch x to y" commit and message, but also the commits themselves that were brought along with the merge from the source branches. And the messages for those commits fail the rule on push, not the actual merge message.

After you do the merge locally, before trying the push, run: git log origin/master..HEAD (replace master with your own target branch). This will show you what that push is actually sending to the origin - your unpushed commits (plural) of the merge. The last commit (first in the output) would be the metadata that registers the merge, and the rest, all the commits you merged from the source branch. Check those messages and see if they would pass the rule or not.

This of course would only apply to merge and rebase merges. Squashed merges should theoretically pass the rule as it would only contain a single commit with a single message. But in our case, squash isn't a feasible option.

I know that the above is the culprit because I tested the same rule with a merge from a more recent branch, with clean commit messages (all after we enforced the rule, or just developers respecting a verbal rule not set in stone), and had no problem pushing the merge and passing the rule.

For the moment, we'll allow bypassing the rule for the older feature branches (+ main), and only when all those branches would get finally merged in main (and then deleted / archived), we could fully enforce the rule, as all commit messages would be clean and would pass the rule.

[patrick-knight]

Hey @Mihai-IDP when's the last time you tested out this flow?

We made a few changes last week around how historical commits are evaluated in rules.

[Mihai-IDP]

We did that yesterday, just before writing the above.

[patrick-knight]

Hate to be that person, but can you try it again and let me know if you are still seeing the same behavior. The feature to remediate this was a slow rollout and did not hit everyone until late yesterday.

[Mihai-IDP]

Hi Patrick, I've retested merging today, same issue. "Merging is blocked", same rule that doesn't pass about the commit message regex: