search

phpro / grumphp

A PHP code-quality tool
MIT License
4.11k stars 429 forks source link

Don't do commit message checks on merges? #1099

Closed delboy1978uk closed 11 months ago

delboy1978uk commented 11 months ago
Q A
Version latest
Bug? no
New feature? no
Question? yes
Documentation? no
Related tickets

Is there a way of NOT doing the Git Commit Message checks when doing a merge? It works great for us on our ticket branches, and we also have a hook adding the ticket number automatically from the branch name, but when we are on develop or master, there is no ticket number, so when we merge it doesn't like it!

veewee commented 11 months ago

That could be a nice addition to the task. There is some sense of merge commit detection already, but we could add a flag to skip the task on merge commits. Care to give it a try?

delboy1978uk commented 11 months ago

@veewee sure, can you point me to the code I should be looking at?

veewee commented 11 months ago

The code for this specific task is located here: https://github.com/phpro/grumphp/blob/v2.x/src/Task/Git/CommitMessage.php

Currently, there is a merge matching mechanism for skipping validation on scope conventions: https://github.com/phpro/grumphp/blob/d0e23de45dbea092b244894cf3ee97844c0b7256/src/Task/Git/CommitMessage.php#L366-L384

It might make sense to extract these merge patterns into a separate function so that it can be used to determine if the task needs to run at all. You could use that same check to skip running the scope conventions.

That specific task is becoming a bit messy because of historical reasons and is open for improvements :)

delboy1978uk commented 11 months ago

Thanks @veewee I'll have a look! BTW I see you're in Belgium, me too! (ik woon al 7 jaar in Leuven)

veewee commented 11 months ago

Nice :) (Ik ben van de Kempen!)

delboy1978uk commented 11 months ago

I'm now able to merge without a JIRA number on a merge! But let me know if I should move that is merge commit check further up or down the chain of events, I allowed it to still go through the first rules, like not empty message etc. Also, you mentioned a flag setting so we can have the merge skip enabled or disabled, how/where do we go about adding the setting?

delboy1978uk commented 11 months ago

Of course, I have broken your tests in doing this :P But I'll wait for your feedback first before continuing work!

veewee commented 11 months ago

Cool. Let's continue the discussion in the PR.