gnustavo / Git-Hooks

Framework for implementing Git (and Gerrit) hooks
http://search.cpan.org/dist/Git-Hooks/
41 stars 17 forks source link

Support for logical OR in plugins, i.e. one of these must succeed #76

Open boudekerk opened 1 year ago

boudekerk commented 1 year ago

Within our company there are two Jira instances (merger/acquisition), and originally different teams used different instances, so we copied and renamed the CheckJira plugin for the second instance.

Now we're in the situation that for some projects multiple keys, from different instances should be allowed. We can probably solve that by using the CheckLog plugin, but the log messages become very unclear to users.

githooks.checkjira1.matchkey=\b[A-Z][A-Z]+\d?-\d+\b
githooks.checkjira1.require=false
githooks.checkjira2.matchkey=\b[A-Z][A-Z]+\d?-\d+\b
githooks.checkjira2.require=false
githooks.checklog.title-match=(\b[A-Z][A-Z]+\d?-\d+\b)|<CUSTOM COMMIT MESSAGE>

It would be nice if somehow multiple Jira instances could be supported with an option of requiring a valid jira key from one of the two.

boudekerk commented 1 year ago

One thought I had was perhaps a separate property which could have a condition specifying which plugins should (or shouldn't?) match.

Something like this for the above case:

githooks.checkjira1.matchkey=\b[A-Z][A-Z]+\d?-\d+\b
githooks.checkjira1.require=true
githooks.checkjira2.matchkey=\b[A-Z][A-Z]+\d?-\d+\b
githooks.checkjira2.require=true
githooks.checklog.title-match=<CUSTOM COMMIT MESSAGE>

githooks.eval=(checkjira1 || checkjira2 || checklog)

Then that would also solve the #58 issue I think, as the logging would be a lot clearer.

Maybe a bit simpler might be adding something in the plugin string, like ! for disabling. | could be prefixed. i.e.

githooks.plugins=CheckLog CheckJira1|CheckJira2

In this case CheckLog would always have to match, and either CheckJira1 or CheckJira2

boudekerk commented 1 year ago

Just realised that the initial config won't even work for us. It will check the key against both instances, so one will always fail.

@gnustavo Do you see a solution?

boudekerk commented 1 year ago

I've implemented the second/simpler proposal in #77.

It would be appreciated if you could consider this.

boudekerk commented 10 months ago

@gnustavo Any further thoughts on this?