Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run pre-commit hook before the prompt #604

Open
dkimot opened this issue Jan 15, 2019 · 21 comments
Open

Run pre-commit hook before the prompt #604

dkimot opened this issue Jan 15, 2019 · 21 comments

Comments

@dkimot
Copy link

dkimot commented Jan 15, 2019

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.

@LinusU
Copy link
Contributor

LinusU commented Jan 16, 2019

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

@dkimot
Copy link
Author

dkimot commented Jan 16, 2019

Then, I'll take a crack tonight.

@dkimot
Copy link
Author

dkimot commented Jan 17, 2019

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.

@ad1992
Copy link

ad1992 commented Jan 20, 2019

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

@LinusU
Copy link
Contributor

LinusU commented Jan 21, 2019

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

@jLouzado
Copy link

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

@junpos
Copy link

junpos commented Mar 12, 2019

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

@dkimot
Copy link
Author

dkimot commented Mar 12, 2019

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.

@jLouzado
Copy link

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

@darktasevski
Copy link

darktasevski commented Apr 23, 2019

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 :(

@ad1992
Copy link

ad1992 commented May 5, 2019

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
Copy link

ad1992 commented Jul 13, 2019

@dkimot @LinusU can I take this up ?

@brandondurham
Copy link

Take it up! Take it up! 😬

@metasean
Copy link

@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?

@savybrandt-zz
Copy link

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

@mihanizm56
Copy link

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

@s14k51
Copy link

s14k51 commented Apr 18, 2020

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.

@evanjmg
Copy link

evanjmg commented Nov 17, 2020

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

@make-github-pseudonymous-again

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

@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.

@make-github-pseudonymous-again
Copy link

make-github-pseudonymous-again commented Feb 26, 2021

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...

@thewanionly
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests