Conventional Commits pre-commit check is not working

[srid]

The second commit in PR #127 has the commit message use same installer - yet the pre-commit check did not catch it, as it doesn't comfort to conventional commits spec.

[srid]

Commit's vanished because I had to force push, but the commit message was literally use same installer.

[srid]

The hook itself works,

However, if I skip it:

And run the flake check:

... it passes ^

[srid]

I don't think the pre-commit module actually adds a flake check.

Also, https://github.com/convco/convco

We can run this manually

[terlar]

Yeah, when running in a flake check it won't have access to .git/COMMIT_EDITMSG and will therefore not be able to check the commit message. The workaround is to use nix run nixpkgs#pre-commit directly on CI. That is what I found as well, although I was using conform.

[srid]

A PR can have multiple commits, so whatever flake check we add must check all of them.

[srid]

But since git history will be available, perhaps it should be run outside of Nix, in github actions.

[shivaraj-bh]

This command checks for all the commits on the current branch, starting from main:

cz check --rev-range main..HEAD

[srid]

Cool. We should also consider #134 when deciding between cz and convco.

[shivaraj-bh]

This command checks for all the commits on the current branch, starting from main:

cz check --rev-range main..HEAD

Can’t do this until NixOS/nix#6900

[shivaraj-bh]

I have made some progress at https://github.com/juspay/services-flake/tree/cz-check, will wait until there is a reliable way to copy .git to the store.

[srid]

Copying .git to the Nix store sounds like a bad idea in general? We only need history of commit messages, not the whole index (much less copy a variation of it every time to the store).

[shivaraj-bh]

We only need history of commit messages, not the whole index

I doubt we can pass this to flake check in a straightforward way.

One hack to make this work will be to use writeShellApplication and run cz check --rev-range main..HEAD via nix run.

[srid]

FYI: this recent commit 5960a2c does not follow the convention, so it is going to be missed when we next generate the changelog. @shivaraj-bh