Skip to content

Enforce good commit messages

Problem

It's common to see an MR that adds 1 simple function with a commit history like this:

* 6e07a4a22f6 (N) - (HEAD -> 44704-improve-project-overview-ui-description, origin/44704-improve-project-overview-ui-description) reset mixins.scss (14 hours ago) <Author1>
* 8ea6b190efc (N) - reset mixins.scss (14 hours ago) <Author1>
* 813ae8f8b9c (N) - refactor and document read_more functionality (15 hours ago) <Author1>
* 019b8066a72 (N) - code style fixes (3 days ago) <Author1>
* 57faca04b1f (N) - fix translated string in _home_panel.html.haml (4 days ago) <Author1>
* 7bd67a5d7a6 (N) - revisions: i18n and button type in _home_panel.html.haml (4 days ago) <Author1>
* ca31dfe4d4b (N) - restore eslint per-line exceptions (4 days ago) <Author1>
* 75bc8922b7f (N) - clean up markup (4 days ago) <Author1>
* bed319b1775 (N) - refactor SCSS for ellipsis usage (4 days ago) <Author1>
* 30627b80e33 (N) - check elements before init (4 days ago) <Author1>
* eee35c5b8aa (N) - add tests for read more functionality (4 days ago) <Author1>
* 03bad6f3fca (N) - update projects feature test to use new selector (4 days ago) <Author1>
* 17de2c3136b (N) - remove comments (4 days ago) <Author1>
* 6534e705138 (N) - move no-new eslint exception to top of file, to appease prettier (4 days ago) <Author1>
* 85f896ab3ba (N) - add functionality to expand project description on phone/tablet (4 days ago) <Author1>

It's also common that a maintainer misses this and merges the MR with this commit history.

Which means that master is starting to look like

git_commit

Which makes it super hard to understand exactly why something was changed or to each MR it belongs to.

Solution

  1. Add this guidelines on how to create commit messages into our development documentation:
Separate subject from body with a blank line.
Limit the subject line to 50 characters.
Capitalize the subject line.
Do not end the subject line with a period.
Use the imperative mood in the subject line.
Wrap the body at 72 characters.
Use the body to explain what and why vs. how.

  1. Instruct every maintainer to check for this before merging it

  2. Because 1 and 2 will most likely be missed, use Danger to enforce it

@meks @rymai what do you think?