commitizen / cz-cli

The commitizen command line utility. #BlackLivesMatter
http://commitizen.github.io/cz-cli/
MIT License
16.66k stars 548 forks source link

Run pre-commit hook before the prompt #604

Open dkimot opened 5 years ago

dkimot commented 5 years ago

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 commented 5 years ago

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

dkimot commented 5 years ago

Then, I'll take a crack tonight.

dkimot commented 5 years ago

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 commented 5 years ago

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

LinusU commented 5 years ago

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

jLouzado commented 5 years ago

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

junpos commented 5 years ago

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

One naive solution I can think of is that

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

dkimot commented 5 years ago

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 commented 5 years ago

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:

darktasevski commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@dkimot @LinusU can I take this up ?

brandondurham commented 5 years ago

Take it up! Take it up! 😬

metasean commented 5 years ago

@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 commented 4 years ago

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

mihanizm56 commented 4 years ago

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

s14k51 commented 4 years ago

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 commented 3 years ago

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

make-github-pseudonymous-again commented 3 years ago

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.

make-github-pseudonymous-again commented 3 years ago

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 commented 1 year ago

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.