Git-hooks configuration

[dpagini]

Would there be any consideration given to the ability to enable/disable the git-hooks through a setting, maybe in project.yml or a custom phing build file?

Ideally, I would like the ability to have more flexibility around the git-hooks. I could see wanting to disable only the commit-msg git hook, or maybe providing my own commit-msg file altogether.

The setup today makes it a little difficult and it is more "all or nothing". Maybe instead of just sym-linking .git/hooks -> vendor/acquia/blt/scripts/git-hooks, we could symlink to the individual files, and you could override each file location with a project specific file...?

build.yml could have:

git-hooks:
  - commit-msg: ${blt.root}/scripts/git-hooks/
  - pre-commit: ${blt.root}/scripts/git-hooks/

...then, in my project, I can override those with:

git-hooks:
  - commit-msg: false
  - pre-commit: ${repo.root}/blt/scripts/git-hooks/

Thoughts?

Again, happy to give this a shot and produce a PR, just want to get some feedback first.

[grasmash]

Yeah, I agree that we need more configurability around git hooks. #454 aimed at doing a bit of that, but it may make more sense to go with your approach and allow the entire hook to simply be replaced.

[timcosgrove]

You could disable the target, if you don't want it: http://blt.readthedocs.io/en/8.x/readme/extending-blt/#disabling-a-target

You can also override BLT's handling of githooks entirely, now, via Phing tasks & overrides loaded via the 'import' property in project.yml. So, specify an XML file via that property and then create your own setup:git-hooks task that does what you want.

We're doing this a lot for places where we have different needs than BLT is accounting for. Though, agree that the location of the git-hooks should be configurable.

[dpagini]

Hmm... that would actually work for me, but I actually want to keep the PHPUnit task, and remove the commit message task, so it might make sense to split those up as two different phing targets if that approach were going to work with me. Maybe the solution is a combination of both suggestions?

[grasmash]

Which Phing target needs to be split?

[dpagini]

setup:git-hooks could split into setup:git-hooks:pre-commit and setup:git-hooks:commit-msg so that only one could be disabled, if you wanted to keep the other. (Selfishly... I want to keep code sniff, but disable, or edit, the commit message hook).

The thing that has stumped me a little on this is the PHPUnit GitTest, I believe it's called. It seems even if I disable a target (I haven't tried just flat out disabling), that this test will still run, and cannot be configured or overridden.

[grasmash]

Oh I see. Good point about the test, that will fail. As a rule I think it's bad to add conditionals to unit tests, but we're not exactly using PHPUnit for a unit test in this case. We could simply check to see if the git hook has been disabled before running the assertion.

[grasmash]

Another possibility is to push #470 to the finish line and exclude GitTest from your CI build using -Dphpunit.excludeGroups=git

[dpagini]

@grasmash I'm still unsure how, once I disable a git-hook (mainly commit-msg), or use a custom git-hook, how I am supposed to tell BLT not to run those git-hook PHPUnit tests that are in acquia/blt? Does this warrant a new issue to be created?
Is this #470 still (I'm not completely following that thread, TBH)?