Skip to content

Conversation

@sethidden
Copy link
Contributor

@sethidden sethidden commented Mar 23, 2022

STOP - Actually check out to the branch, run "yarn" and see if commiting to the branch the way you usually do it (maybe from within IDE? maybe terminal?) behaves to your liking

We have a git hook that launches when you type git commit, the problem up until now is that it just errored out during launch, so it didn't change the way git commit works originally. Here I'm fixing it so it works as intended by the original author of the commit that added it.

But I suspect that the sudden change of how git commit works may break the workflows of contributors and devs who commit outside the terminal. Basically, please check out to this branch and try to commit something the way you usually do it and see if that isn't annoying. You can commit from inside Webstorm, using the terminal, or from Vim (like I do and where this PR breaks things, so I'll have to commit from the terminal if this is merged)

I'm super open to removing this hook altogether

@sethidden
Copy link
Contributor Author

Screen.Recording.2022-03-23.at.14.41.30.mov

a

bloodf
bloodf previously approved these changes Mar 23, 2022
@roziscoding
Copy link
Contributor

I'm also in favor of removing this hook, or maybe finding a way of making it optional. If people don't run husky install then the hook shouldn't be created, right?

Anyway, commitizen doesn't actually guarantee good commit messages, commitlint does that, so the hook is more of a "cosmetic" feature.

@sethidden sethidden marked this pull request as draft March 23, 2022 16:09
@sethidden
Copy link
Contributor Author

Yeah I was thinking about removing this hook, but keeping cz in package.json so people can npx cz if they don't remember all the available feature:, perf: things by heart. The biggest inconvenience is that commitlint will fail in the PR check and they'll have to fix it.

@Frodigo
Copy link
Contributor

Frodigo commented Mar 24, 2022

I am either voting for removing that pre-commit hook and as you mentioned @sethidden - keeping cz in package.json is a good idea.

before this commit running git commit ran commitizen instead of showing
the traditional git commit message prompt

doing that was too much of an intrusion into the workflow so we've
decided to keep cz, but make it optional to use instead of required
@sethidden sethidden force-pushed the M2-343-fix-husky-pre-commit-hook branch from 24619bf to 66ca107 Compare March 24, 2022 07:36
@sethidden sethidden marked this pull request as ready for review March 24, 2022 07:36
@sethidden sethidden changed the title fix: prepare-commit-msg using wrong command fix: remove commitizen prepare-commit-msg git hook Mar 24, 2022
@sethidden sethidden merged commit cb11323 into develop Mar 24, 2022
@sethidden sethidden deleted the M2-343-fix-husky-pre-commit-hook branch March 24, 2022 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants