Skip to content

Conversation

@12rambau
Copy link
Contributor

@12rambau 12rambau commented Apr 21, 2023

commitizen is a commit-msg hook, it won't be installed by default. Using the default_install_hook_types makes sure that all the hooks used in this file are installed when running:

pre-commit install

related to #257

commitizen is a commit-msg hook, it won't be installed by default. Using the `default_install_hook_types` makes sure that all the hooks used in this file are installed when running:

```
pre-commit install
```
@12rambau
Copy link
Contributor Author

@maartenbreddels note that the error raised from the CI is completely unraleted from my PR it's ready to be merged.

@12rambau
Copy link
Contributor Author

@mariobuikhuizen I've rebased on master as well and it runs like a charm

@12rambau
Copy link
Contributor Author

12rambau commented Sep 7, 2023

@mariobuikhuizen could you merge this one ? It's harmless for the rest of the code and it simply ensure that the commitizen hook is installed by default (commit-msh is not in the default list).

I'm constantly forgetting to add it when I work in a github devcontainer, it would make my life easier (and more transparent for someone that doesn't know pre-commits well)

@mariobuikhuizen
Copy link
Collaborator

I need to evaluate this tool before proceeding, but unfortunately, I can't prioritize it at the moment.

@12rambau
Copy link
Contributor Author

12rambau commented Nov 7, 2023

requesting review of this PR again: it changes nothing apart for pre-commit users. 2 hooks are already set-up but without this line, only one is installed by default when running "pre-commit install". As you have already completely adopted the conventional commit it will not even influence the way you code.

@mariobuikhuizen
Copy link
Collaborator

There are 3 issues I have with using this tool:

  1. You'll need to have node installed
  2. Not all commits in PR's will end up being merged because of squashing. So requiring strict rules for commits that probably get squashed is not ideal.
  3. It probably doesn't work with git rebase -i ...

Since you need it for the github devcontainer, would it help if you could configure you own custom dev container in .devcontainer/12rambau in this repo? See: https://docs.github.com/en/codespaces/setting-up-your-project-for-codespaces/adding-a-dev-container-configuration/introduction-to-dev-containers#creating-a-custom-dev-container-configuration

@12rambau
Copy link
Contributor Author

Then let's turn the table and drop commitizen pre-commit hook alltogether.

My point is if it's there it should be hard to miss to be useful. If you prefer not to use it, I'm nobody to enforce it but in that case let's drop it from pre-commit.

@12rambau
Copy link
Contributor Author

any news ?

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.

2 participants