Considering enforcing conventional commit specification

[Fdawgs]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the issue has not already been raised

Issue

Whilst the release notes are great at showing what changed and who committed what, the individual items are a mixed bag, and make it difficult to skim read.

As an example from v4.19.0:

From a glance, I can't tell what that pluginName will be exposed in FastifyInstance change is. Is it a bugfix? Is it a new feature? Was it just the documentation that was updated? The other two items in the list I can clearly see what type of change they were.

I think this could be remedied by enforcing the use of the conventional commit specification using the Angular convention with commitlint in a husky hook and/or the commitlint GH Action.

[Uzlopak]

I thought it is not the commit message but the pull request title, that github is using to describe the changes?!

Maybe a workflow which checks the title of the pull request?

[Fdawgs]

I thought it is not the commit message but the pull request title, that github is using to describe the changes?!

It is, but the PR title is usually generated from the first commit message (unless there's multiple commits in the PR); see Fdawgs/fastify-disablecache#228

[Uzlopak]

to be honest, i personally dont like commit message linting. It is quiet annoying to get it right. Also you need it for the first commit only, but you would be forced to do it on every follow up commit.

We could fork something like https://github.com/deepakputhraya/action-pr-title and modify it for our needs

[Fdawgs]

We could fork something like https://github.com/deepakputhraya/action-pr-title and modify it for our needs

That looks a lot better! And you're right, the PRs are all squashed anyway, so it's only the PR title that matters.

[Uzlopak]

I will fork and modify the action.

[jsumners]

I loathe conventional commit. It was a source of constant frustration at my last gig.

[metcoder95]

+1 on the suggestion, as well on the thoughts from @Uzlopak about having a GHA instead of a git hook that validates that.

[mcollina]

I think we should validate the PR title, not the individual commit. That's what ends up in the release notes anyway.

[Uzlopak]

check this:
fastify/action-pr-title#3

I modified the action for our needs imho:

edit the pull request title to something without the prefix chore, fix or feature, and it fails. change it to match the prefix and it gets green.

I assume, that by modifying the dependabot commit-message we can prefix it with chore. I dont know if the automerge fails, because it maybe doesnt realize it is dependabot. Should be investigated.

[metcoder95]

You can configure dependabot to prefix the commit title. Although, I've never needed to configure it this way, most of the time it just works like this.

I think this could be remedied by enforcing the use of the conventional commit specification

We should also communicate this within the CONTRIBUTING guide to formalize these guidelines for new contributors.

[Uzlopak]

I am with @jsumners on this one. I hate commit message rules. They are just annoying. And when we squash merge we potentially cant validate them.

[Eomm]

I try to add the right label to the PR every time.

I would add an action that:

• on label added/removed: rename the PR title adding the correct prefix
• when the title does not match any prefix the CI is read

so, the squash&merge operation will be smooth and easy

[Eomm]

We should also communicate this within the CONTRIBUTING guide to formalize these guidelines for new contributors.

The markdown linter is already a pain - I would not add more stress to contributors IMHO

[Uzlopak]

It is not clear how to solve that.

If I add a label typescript I prefix title with types:.
If I add a label documentation I prefix title with docs:.

What if I add the label typescript and documentation? How do I solve this gracefully?

[climba03003]

To reduce the burden of new contributor, we can actually do the job ourselves.
Before squash and merge, Github allow us to update the PR commit message which is used to generate the changelog. This is what I am doing right now.

[jsumners]

From a glance, I can't tell what that pluginName will be exposed in FastifyInstance change is. Is it a bugfix? Is it a new feature? Was it just the documentation that was updated?

What is the problem with reading the details of items that stand out to you by clicking to the linked issue/pr?

The other two items in the list I can clearly see what type of change they were.

Are such labels guaranteed to be accurate? Hint: nope. Note that Node core doesn't bother with this arbitrary labeling either https://github.com/nodejs/node/blob/8a725c7d026b8d12b1fae597e93f90367c1da2fa/doc/changelogs/CHANGELOG_V18.md#commits-1

[Uzlopak]

Well I created that action https://github.com/fastify/action-pr-title in fastify org. Do we want to use it, or should I delete the repo, before somebody external wants to use it for its non-fastify-org repo :D...

[mcollina]

let's try it, we can always drop it. Would you mind setting it up in this repo?

[Fdawgs]

Thanks @Uzlopak, sorry been a tad busy with work so haven't had a chance to revisit this recently. ☹️

[jsumners]

We have had a couple of releases with this enforced. Is it bringing any value?

[Eomm]

yes, it does and the release message is slightly better IMHO:

with: https://github.com/fastify/fastify/releases/tag/v4.25.0
without: https://github.com/fastify/fastify/releases/tag/v4.18.0

[jsumners]

@Eomm in your view, what is the improvement between those two?