[rush] Add support for git hooks

[nchlswhttkr]

Resolves #821

Feedback on this would be welcome, I'm not sure how to test this during runtime.

At the moment, I'm just copying over any files in common/git-hooks to the git config, so stricter rules could be applied if needed.

Is there any need to worry about clearing the .git/hooks directory before writing to get rid of outdated scripts? FileSystem.copyFile should overwrite automatically, but that won't get rid of old files if a replacement does not exist.

Happy hacktoberfest!

[octogonz]

@nchlswhttkr cool, thanks for making this PR! :-)

My main feedback is that this should probably be a repo-wide configuration, rather than a command-line option. At least, in the use cases that I've observed, the repo maintainer would want to set this up for all developers (e.g. to normalize their code using Prettier), rather than it being an "opt-in" feature that happens when they install.

One way to do that would be via the rush.json config file. Although in the original issue it occurred to me that the presence of the common/git-hooks/ folder might be sufficient. #Resolved

[nchlswhttkr]

I agree with that. I think having a common/git-hooks/ folder is sufficient a repo to opt-in, so long as the behaviour is documented.

For now, I can change to logic to run by default and log when it detects and copies over any new/updated git hooks. #Resolved

[octogonz]

Is there any need to worry about clearing the .git/hooks directory before writing to get rid of outdated scripts? FileSystem.copyFile should overwrite automatically, but that won't get rid of old files if a replacement does not exist.

Yes, I think this makes sense, for a couple reasons:

• If the repo maintainer deletes an unneeded hook, but the file remains in the person's .git folder, then when that hook is invoked it may fail because the script it's pointing to no longer exists. That would cause a lot of headaches. So the default behavior should be to delete.

• Some people may want to install their own additional Git hooks, which are no managed by Rush. In this case they need an config file or environment variable to override the default behavior. There are some design questions. If/when someone asks for that, they should have the burden of figuring that out. :-)
  #Resolved

[octogonz]

One edge case would be if NO hooks are registered with Rush. In this case, it might be safer to do nothing, rather than deleting all the Git hooks. (?) #Resolved

[nchlswhttkr]

I've updated the PR accordingly, for now it clears any existing hooks before doing an install.

Having the /common/git-hooks folder is still what triggers the opt-in behaviour, but it will warn and skip if the folder is empty.

For now, Rush is just providing a central location to store and manage repo-wide git hooks. If we want to extend beyond this with more configuration options, that discussion can occur another time when we know the requirements better. #Resolved

[octogonz]

Awesome! @cliffkoh @kenotron would you guys want to try this in the OUIFR repo?

It's probably more efficient than Husky. If I remember right, Husky executes shell scripts for every single Git event, regardless of whether there is actually a handler or not. #Resolved

[octogonz]

Hey @nchlswhttkr, sorry it took a while to follow up. We wanted someone to test it out in a real repo before we merge the feature. I just chatted with @natalieethell who is going to give this a try in the office-ui-fabric-react project, which uses Git hooks. Then we'll finally merge it. Thanks for your patience! #Resolved

[nchlswhttkr]

No worries 👌 #Resolved

[natalieethell]

Just finished up trying this out in the office-ui-fabric-react project, and it seems to work as expected. Everything in .git\hooks is deleted and/or replaced with the files in common\git-hooks during a rush install.

Additionally, if common\git-hooks is empty, nothing changes in .git\hooks, as expected, and a warning is displayed. #Resolved

[octogonz]

Awesome, thanks a bunch @natalieethell ! I'll merge this and include it with the next Rush release #Resolved

[octogonz]

[octogonz]

@nchlswhttkr FYI this was published as part of Rush 5.5.1

I also wrote some documentation here: https://rushjs.io/pages/maintainer/git_hooks/

Thanks again for implementing this!

[nchlswhttkr]

You're welcome, have a good one!