add conventional commit check

[ctron]

No description provided.

[jcrossley3]

Is this going to require me to write "chore"?

[ctron]

Is this going to require me to write "chore"?

Yes.

[jcrossley3]

The goal is to facilitate automatic creation of release notes. How does you forcing me to write "chore" on every commit that should not be a part of the release notes help with that?

[ctron]

It automates the process of checking and reminding you.

If I understand the poll correctly 4 voted in favor of this on #1355 … maybe I don't.

[jcrossley3]

A reminder is fine. A failed check is unacceptable.

[helio-frota]

A reminder is fine. A failed check is unacceptable.

I tend to agree with that because of cache, if fails and we need to re-run the review or whatever it will cause a new run according to recent observations...

in the other hand we have failures with clippy and format, the format is solved with 1 command which looks like the chore: situation.. it causes small doses of f.. feelings, I agree.

yeah I'm ➕ for reminder only if possible

[helio-frota]

@jcrossley3 @ctron :
https://github.com/wagoid/commitlint-github-action?tab=readme-ov-file#failonerrors ? true-false ?

[ctron]

A reminder is fine. A failed check is unacceptable.

A reminder without someone enforcing it is kind of useless. We have that today. And it's not working.

[ctron]

A reminder is fine. A failed check is unacceptable.

I tend to agree with that because of cache, if fails and we need to re-run the review or whatever it will cause a new run according to recent observations...

in the other hand we have failures with clippy and format, the format is solved with 1 command which looks like the chore: situation.. it causes small doses of f.. feelings, I agree.

yeah I'm ➕ for reminder only if possible

I am not sure how "cache" fits in here. If that workflow fails, you need to push a new commit anyway. Which will trigger a new build.

And as you mention, we also do enforce cargo fmt, and not just "recommend" it.

[ctron]

@jcrossley3 I created #1375, so that you can come up with a better plan.

[helio-frota]

And as you mention, we also do enforce cargo fmt, and not just "recommend" it.

yes that is correct. Feedback about JS --> I prefer cargo fmt, to be honest. I don’t want to go back to fighting with different formats for no reason 👍

I am not sure how "cache" fits in here. If that workflow fails, you need to push a new commit anyway. Which will trigger a new build.

that was something I saw yesterday, although not 100% focused on failing builds during the investigation, so I can't affirm with a proper test/show case

[helio-frota]

@ctron we (at previous team) used husky as git hook to run tests before pushing nodeshift/new-module-starter@b2ffc2a

not sure if rust has something like that to apply to git commit before send a PR 👍
that solves the problem with the cache ( in case that problem exist )

edit: https://github.com/swellaby/rusty-hook and/or https://crates.io/crates/cargo-husky

not sure if works for git commit messages prefix validation

Edit: I don't like pre-git hooks, just sharing, and I have no problems with chore:

[JimFuller-RedHat]

LGTM

[JimFuller-RedHat]

@jcrossley3 do you feel so strongly about this that you want to overrule the entire team ? I am all for any individual feeling strongly ... but having a hard time seeing the chore in writing 'chore-' or put another way there is probably clever ways to automate that chore ;) or maybe more importantly I am missing some other potential issue with this approach ?

[jcrossley3]

@jcrossley3 do you feel so strongly about this that you want to overrule the entire team ? I am all for any individual feeling strongly ... but having a hard time seeing the chore in writing 'chore-' or put another way there is probably clever ways to automate that chore ;) or maybe more importantly I am missing some other potential issue with this approach ?

I feel strongly about writing good commit messages. And I don't believe conventional commits are sufficient for doing so. And in some ways, particularly "chore", I feel they diminish the quality of the message.

But I like that "fix:" and "feat:" drive our release notes. And I'm not opposed to you or others adopting the convention whole hog. I just don't want it enforced/required.

[JimFuller-RedHat]

This PR is not a comprehensive solution to writing a 'good commit message' - it is incremental and mainly helps generate release notes, as well as categorise commit messages ... I do not think accepting it blocks any future efforts in improving commit message quality. I do not see how writing 'chore' damages a commit message ... its just a simple tag (which we could argue be another word).

I am inclined to merge this ... mostly because the team mostly gave a thumbs up - but if you feel strongly I will close this PR.

[jcrossley3]

Thanks! I really don't want to be required to write "chore: " on every commit that shouldn't be in the release notes.

And now I worry that we'll have a bunch of "fix: typos" commits showing up in the release notes. 😉

Whether we require "chore: " or not, we need to consider release notes for every single PR review going forward.

[helio-frota]

Thanks! I really don't want to be required to write "chore: " on every commit that shouldn't be in the release notes.

I found a balance/middle ground configuration 👍

add this to root project dir: commitlint.config.mjs (based on https://github.com/wagoid/commitlint-github-action/blob/master/commitlint.config.mjs#L17 )

export default {
  extends: ['@commitlint/config-conventional'],
  plugins: ['commitlint-plugin-function-rules'],
  rules: {
    'type-empty':[0, 'always'],
    'subject-empty':[0, 'always']
  },
}

edit:

➜  cascade-feat git:(foobar) convco changelog -s --max-majors=1 --max-minors=1 --max-patches=1 -n > changelog.md

➜  cascade-feat git:(foobar) git log --oneline
f335f4a (HEAD -> foobar, origin/foobar) superperformance: aaa
ec5c2aa .
f2b2970 tests
6e8a060 tests
a3882fd tests
80df18b tests
5c621f7 tests
6aadced tests
dbc73d9 tests
7d7f6a5 tests
32dc820 style: fix typo
2c67f3d style: adding typo
ceab20f feat: add changelog
de56f7b aaa
d0abc1a no chore
8bdc34a (origin/main, origin/HEAD, main) chore: foo
35641c8 test
80c7b75 foobar
5448ec2 bar
ea36f90 foo
0abee4f Initial commit
➜  cascade-feat git:(foobar) cat changelog.md
# Changelog

## Unreleased (2025-03-03)

### Features

* add changelog (ceab20f)

[helio-frota]

This is not a balance/middle ground ^... the main issue is the chore:, we end up only with some side-lint-benefits like showed in the screenshot superperformance: prefix for example

[helio-frota]

I am inclined to merge this ... mostly because the team mostly gave a thumbs up - but if you feel strongly I will close this PR.

+1

we can try to ask then..."could you please add feat: ? as you are adding a new feature to the project, thanks 👍 "

And now I worry that we'll have a bunch of "fix: typos" commits showing up in the release notes.

and do the same here "could you please change to style: prefix?, as conventional stuffs says blank spaces are style, then probably typo is in the same boat, thanks 👍 "