Allow fixup! commits (again?)

[vitaly-fanaskov-r]

Hi!

I read carefully through the previous discussion about the subject and decided to raise the matter again. Strictly following conventional commits is a good thing, but during the code review process you might be using "fixup!" commits to make the process simpler for the reviewer. It means that --no-verify is not an option anymore. And when I'm adding "fixup" to the "args" list it still requires ":".

In light of this, it'd be nice to have an option like this or similar:

    hooks:
      - id: conventional-pre-commit
        stages: [commit-msg]
        args: []
        allow-fixup-commits: true

It should be easy to use a simple regexp like this ^(fixup! +) to check it or even a method like startswith.

[thekaveman]

Hey there @vitaly-fanaskov-r, thanks for the request.

As you note, there has been much discussion here on supporting fixup commits. Please see this comment for more context on why your proposed config won't work as-is (TLDR; pre-commit options can only be passed through args): https://github.com/compilerla/conventional-pre-commit/issues/4#issuecomment-1486432897

In light of the continued interest in supporting these types of commits, we would welcome a PR that incorporates @JP-Ellis thoughts from the comment linked above.

[vitaly-fanaskov-r]

Cool, thank you!

It seems that this section tells us that the arguments will be just passed as is. I see that there is already --force-scope we can do the same for --allow-fixup-commits. At the same time, it's worth considering a more generic approach, i.e. passing a dictionary or a set of extra options. I.e. 'config={"foo": "bar", }'.

[thekaveman]

I see that there is already --force-scope we can do the same for --allow-fixup-commits.

I think that this is the right approach for now 👍

it's worth considering a more generic approach, i.e. passing a dictionary or a set of extra options. I.e. 'config={"foo": "bar", }'.

Worth considering but maybe a little more than we need at this point. We can keep it on the backlog for sure if configuration starts to get too unwieldy from the .pre-commit-config.yaml

[vitaly-fanaskov-r]

I think that this is the right approach for now

Sounds like a plan. Do you have time to implement it? I think I'll be having some time this week and will be able to make a PR.

[thekaveman]

Do you have time to implement it? I think I'll be having some time this week and will be able to make a PR.

I won't be able to do any work myself this week, but I'd be happy to review a PR whenever you are able to submit.

[thekaveman]

@vitaly-fanaskov-r I've been thinking about this a little more...

It would probably be more comfortable for you and others to not have to use a new args to be able to use --fixup commits.

At the same time, users that do not currently use --fixup commits will not notice a change either way.

What if instead we made --allow-fixup-commits the default behavior (for users like yourself), and create an option like --strict or something that turns it off, for users that truly want to outlaw such commits?

[vitaly-fanaskov-r]

@thekaveman I agree, it sounds much better!

[thekaveman]

v2.4.0 is released! Thank you @vitaly-fanaskov-r!

Available in pre-commit:

- repo: https://github.com/compilerla/conventional-pre-commit
  rev: v2.4.0
  hooks:
    - id: conventional-pre-commit
      stages: [commit-msg]
      args: []

And on PyPI: https://pypi.org/project/conventional-pre-commit/2.4.0/