Gatsby: Git Commit Message Convention

[fk]

Browsed around a little in the context of #3750 and stumbled upon

• https://github.com/vuejs/vue/blob/39d58e521a4a6bc4c0d42f17453421ae155ec04f/.github/COMMIT_CONVENTION.md
• https://github.com/angular/angular/blob/3f5a3d6ea1d20d8d6d662d0912a875b68db5c3b1/CONTRIBUTING.md#-commit-message-guidelines

Do we need/want those?

[m-allanson]

If these means Gatsby will have an up-to-date changelog, I'll give it a big 👍.

But I think the convention is only useful if every new commit follows it, which probably means having some tooling to enforce it.

[KyleAMathews]

Yeah, we really need to get a CHANGELOG going again. Which will mean a bot to enforce the convention + https://github.com/gatsbyjs/gatsby/pull/2198

[jmoujaes]

Found this example of using git hooks to enforce commit messages. It may be something to consider when implementing this.
https://git-scm.com/book/en/v2/Customizing-Git-An-Example-Git-Enforced-Policy

[vprasanth]

This might help, https://www.npmjs.com/package/commitizen. Might get annoying though. There would probably still be a need for some kind of up upstream check though, like a bot.

[jlengstorf]

I've set this up when I was at IBM with husky and precommit hooks. With solid conventions we could Auto-generate not only the changleogs but even package versioning/publishing with semantic-release. As a process nerd, I :heart: this.

[vprasanth]

@jlengstorf did you also use commitizen with that setup or just husky?

[huchenme]

We did use lerna publish --conventional-commits to publish with commitizen to enforce the commit message format, it works very well. Shall I implement this and submit a PR for that?

[jlengstorf]

@vprasanth we also used commitizen

@huchenme I worry that the trade-off might be too big. For newer contributors, Git is already hard, so having commits rejected could be really confusing and discouraging.

Another option might be to control this on the maintainer side: only use squash commits for PRs, and require commitizen-friendly commit messages. This would still give us changelogs and automatic deploys and so on, but wouldn't add the extra cognitive load for contributors.

@m-allanson, what do you think?

[pieh]

@jlengstorf I like this idea very much. After DCO expiriment we learned that additional requirments like this would block a lot of contributions and maintainers for sure can handle commitizen-friendly commit messages when we merge PRs

[huchenme]

@jlengstorf do you mean let code reviewer to manually check whether the commit message is in the correct format?

[jlengstorf]

@huchenme I made a video of what I mean: http://cl.ly/r2KK

Using squash merges, the original commit message is replaced by the person merging a PR. This means we only have to train contributors with merge privileges instead of _everyone_ who wants to contribute to Gatsby.

[huchenme]

@jkengstorf I see what you mean, but it won’t be good for all cases, if a PR got a breaking change in one sub package and some other non breaking changes in another sub package, squash and merge will create a single BREAKING CHANGE and make a major version bump for both packages, some time it is necessary to split commits, are we able to do that in GitHub?

[KyleAMathews]

That scenario would be extremely rare and if so, we could probably manually edit the changelogs.

[jquense]

I've been switching most of my repo's over to semantic-release recently, and thought i'd share what i learned so fa. I really enjoy it but it's not clear to me how the PR flow should work (still) in projects with lots of contributors. I've found that commitizen is to onerous and commit linting in general is just not feasible for a smooth workflow. They do have a cli to help folks but its so far outside the workflow of every other project i don't think you'd get many people to use it. "Squash and Merge` is great when folks don't follow the guidelines but it's also a bit of a footgun, if a commit that should trigger a release gets into master it's annoying to fix it, since you'd have to rebase and force push, or add a new dummy commit, or release by hand, which means you miss out on all the github release stuff and PR bot notifications.

Overall i think it's worth doing, but we _need_ a workflow that isn't dependent on contributors always formatting their commits correctly (see the recent DCO conversation as well), as well as clear guidelines for mantainers on how to merge PR's, and how to fix mistakes

[m-allanson]

I agree that any unusual workflow is going to be a barrier to entry. It'd be nice if we could allow contributors to format their commits however they like.

But enforce a set of conventions for merge commits. I guess this'd need to be done via the GitHub UI, and I don't think there's any way to do that.

[jlengstorf]

To @jquense's point, I think the trade-off of low-effort for contributors vs. footgun of core maintainers forgetting to do something is worth it. @kentcdodds has a convention for manual releases that I really like — it's better if we get the commits right the first time, but this manual release at least gives us the ability to fix the changelog without rewriting history or other messy fixes.

Ideally, I think we’d _encourage_ a standard commit convention, and maintainers can mention/link to it when a new contributor doesn't follow it, but ultimately core maintainers will be the only people with extra chores, which I think is going to ultimately be better for adoption and consistency.

[kentcdodds]

Hi friends :wave:

First off that manual releases thing is awesome. I definitely recommend it if you're automating releases.

Secondly, I would strongly discourage enforcing a commit message format on contributors. For three reasons:

1. It increases the barrier to contributing for new contributors.
2. It's super annoying to have to update a PR just to update a commit message.
3. You can change the commit message when you merge from GitHub's UI anyway!!!!!

Just use GitHub's squash and merge feature and it'll allow you to set the commit message of the squashed commit. So I never even mention the commit message format to anyone who doesn't have merge access because their messages never actually make it to master anyway. I always rewrite it when I merge anyway.

Cheers!

[KyleAMathews]

Due to the high volume of issues, we're closing out older ones without recent activity. Please open a new issue if you need help!

[KyleAMathews]

Oh actually, we're probably going to work on this soon 👍

[mikelax]

Another simple win with low effort would be to enhance the existing Pull Request Template. We just enhanced ours and added a checklist of common tasks. ie here is a copy of our template:

# Pull Request Overview

<!-- Write a brief overview for the PR, and what was addressed -->

## Open Questions or Items to Callout

<!-- Use this area to document any specific questions or areas of concern for the Reviewers -->

## PR Checklist

<!-- Please make sure you have addressed the following before opening a new Pull Request -->

- [ ] Create any needed unit and/or Cypress Tests, all passing :100: ?
- [ ] Create Storybook stories for any applicable Components
- [ ] Run lint commands and clean up any errors :rocket:
- [ ] Reference a Jira ticket number in the description
- [ ] Assign PR to yourself and select ~2 Reviewers
- [ ] Add `work in progress` label if applicable
- [ ] Add `needs db migrations` label if applicable

[jlengstorf]

I also think something like this solves the problem in a general way with very low effort: https://github.com/probot/semantic-pull-requests

[jlengstorf]

I'm closing this issue in favor of #7625. I've added a summary of this conversation to that issue so we don't lose the great ideas here.