cz-cli icon indicating copy to clipboard operation
cz-cli copied to clipboard

Run pre-commit hook before the prompt

Open dkimot opened this issue 6 years ago • 22 comments

I use pre-commit hooks to format my code and check with npm audit for insecure modules. There are times I run through the whole commit prompt only to have my pre-commit hook(s) fail and have to redo my commit message.

I think it would be great if the cli ran pre-commit hooks before the prompt much like it checks that some files have been added to staging.

I'm happy to work on a PR, just curious what the community thinks.

dkimot avatar Jan 15 '19 19:01 dkimot

Yes please, I've run into that a ton of times as well 😁

LinusU avatar Jan 16 '19 15:01 LinusU

Then, I'll take a crack tonight.

dkimot avatar Jan 16 '19 21:01 dkimot

After taking a look at this I found the --retry command in #132 .

While this is almost what I want, if I use --retry on properly formatted code after failing a commit the improperly formatted code gets committed, it does not get updated. While this may be a feature not a bug, I'd vastly prefer the ability to fix formatting and then commit.

dkimot avatar Jan 17 '19 01:01 dkimot

@dkimot @LinusU I still think we should not allow the cli to run if pre-commit hook fails. what do you guys think ?

ad1992 avatar Jan 20 '19 19:01 ad1992

Personally agree, would like to know what @jimthedev thinks though ☺️

LinusU avatar Jan 21 '19 13:01 LinusU

Definitely an issue... if pre-commit hooks fail there's no point in filling out a commit-message.

jLouzado avatar Feb 27 '19 13:02 jLouzado

Yes, I agreed and this's been an issue for my team as well.

One naive solution I can think of is that

  • Manually run a git pre-commit hook, without attempting a commit, bash .git/hooks/pre-commit before running prompter for getting user input,
  • And pass --no-verify options to dispatchGitCommit so that the pre-commit hook will be ignored.

https://github.com/commitizen/cz-cli/blob/master/src/commitizen/commit.js#L47-L62

junpos avatar Mar 12 '19 17:03 junpos

At this point I think it's pretty clear there is community support and I don't think --retry is solving the problem correctly. I'll start on something tonight. I was waiting to see what @jimthedev thinks, as @LinusU suggested, but it seems like they're busy.

dkimot avatar Mar 12 '19 17:03 dkimot

if I use --retry on properly formatted code after failing a commit the improperly formatted code gets committed, it does not get updated

@dkimot what do you mean? After fixing the formatting do you mean that the changes should get automatically staged?

I'd recommend not doing that, you don't want any magic concerning what is staged and what isn't:

  • If a prettier:check is failing, then the user should fix formatting, manually stage the commit and then retry
  • by accident if some patch gets staged that wasn't intended for that commit then it causes confusion

jLouzado avatar Mar 13 '19 07:03 jLouzado

I've managed to resolve this issue somehow, a bit hacky IMO, but it seems to be working fine.

I got rid of husky's pre-commit hook and moved script it was executing to the npm scripts section.

	"scripts": {
		"pre-commit": "yarn lint:ignore-tests && yarn test && lint-staged && git add --all",
		"cmz": "npm run pre-commit && git-cz"
	},
	"husky": {
		"hooks": {
			"commit-msg": "commitlint -E HUSKY_GIT_PARAMS"
		}
	},
	"config": {
		"commitizen": {
			"path": "git-cz"
		}
	}

After git-cz have finished with writing commit message, I'm running commitlint here to see if the commit-msg was okay.

But, I still seem to have an issue when running git commit instead of yarn run cmz the commitlint will run and throw error if developer try to commit with non-semantic commit message, but if he git commit with semantic one the pre-commit script won't run :(

darktasevski avatar Apr 23 '19 14:04 darktasevski

We shouldn't allow the the cli to run if pre-commit hook fails and also if files are not added. Did everyone agree on this ? Is anyone working on this ? Or Can I take this up ?

ad1992 avatar May 05 '19 08:05 ad1992

@dkimot @LinusU can I take this up ?

ad1992 avatar Jul 13 '19 14:07 ad1992

Take it up! Take it up! 😬

brandondurham avatar Jul 26 '19 19:07 brandondurham

@ad1992 && @brandondurham - Any updates on your fixes so that the pre-commit hook will run before git cz starts walking the user through the commit questions?

metasean avatar Aug 15 '19 18:08 metasean

Any updates here? My team just started using commitizen and love it, but having specs or linting run first would be great

savybrandt-zz avatar Jan 03 '20 02:01 savybrandt-zz

Any updates here ? I'm using git cz and have the same question - how to crash cli if pre-commit hook was failed.

mihanizm56 avatar Feb 27 '20 09:02 mihanizm56

I use a similar approach as what @Puritanic mentioned above, and it doesn't need changes to the cli

  1. Remove husky's pre-commit hook
  2. Move the scripts it was executing to a git alias
    git config --global alias.c '!f() { npm run lint && npm test && git cz; }; f'
    
  3. Now, whenever I want to commit, I simply have to run
    git c
    
    or I can bypass pre-commit's scripts with the regular
    git cz
    

I feel this is much simpler as I do not have to remember that I have to run an npm script just to do a git commit, but it would be good if the cli takes care of running the pre-commit hook before showing the questions.

s14k51 avatar Apr 18 '20 20:04 s14k51

Any update on this? It's been almost 2 years

evanjmg avatar Nov 17 '20 11:11 evanjmg

I got rid of husky's pre-commit hook and moved script it was executing to the npm scripts section. https://github.com/commitizen/cz-cli/issues/604#issuecomment-485826445

@Puritanic You can keep husky by adding --no-verify to git-cz. This allows the precommit hook to still run on normal git commit usage. Unfortunately it disables the commit-msg hook too.

Would it not work to temporarily set the git EDITOR variable to something that does cz | "original" EDITOR? Unless that is how cz already works...

Have we got any updates on this? It's been 4 years already. Just started to use commitizen and having this issue fixed would be great.

thewanionly avatar Apr 06 '23 16:04 thewanionly