Git commit style

This is a copy of the git commit style we wrote in Aug 2022

Our codebase is open-source and public, and is one an important part of both our work and public image.

With the increasing number of contributors in projects, it is time to standardize on the style of git commits and processes on our projects.

Proposal

Credit: This proposal is inspired by the ØMQ Contribution Policy

Commit messages become the public record of your changes, as such it’s important that they be well-written. The basic format of git commit messages is:

  1. A single summary line starting with a keyword Feature, Fix, Refactor, Doc or Cleanup and a description of the problem that the commit solves.
    This should be short — no more than 70 characters or so, since it can be used as the e-mail subject when submitting your patch and also for generating patch file names by ‘git format-patch’. If your change only touches a single file or subsystem you may wish to prefix the summary with the file or subsystem name.
  2. A blank line.
  3. A detailed description of your change starting with "Solution: and explaining the solution used in the commit. If your changes have not resulted from previous discussion on the mailing list you may also wish to include brief rationale on your change. Your description should be formatted as plain text with each line not exceeding 80 characters.

Examples:

Fix: API calls fail when server api2 is down

Solution: Use load balancer via `official.aleph.cloud` instead of `api2.aleph.im`. Requests are then load balanced to either `api1.aleph.im` or `api2.aleph.im`.
Feature: API server could not be specified in CLI

Solution: Add CLI argument `--server` that users can use to replace the default API server
Refactor: Synchronous HTTP requests caused performance issues

Solution: Replace `requests` with `aiohttp` and make code that relies on HTTP requests asynchronous.
Cleanup: Code style was not pep8 compliant

Solution: Format the code using `black`, the uncompromising Python code formatter, and add documentation to use it in git pre-commit hook.
Doc: Version number in README was obsolete

A solution is sometimes obvious from the title, mentioning “Solution: Update the readme with the new version number” is pretty useless.

Reactions

Explanation in the Pull Request

From @MHHukiewitz : Having a detailed explanation always occurred to me to be rather part of the PR description. But I see your point how it may improve the reviewer’s understanding of the thought process during the PR.

The issue with relying on the PR description for the explanation is that these are not easily copied outside of the GitHub repository that was used (forks, switching to GitLab / Radicle / Gitea / …).
They also are not available in most IDEs.

Features as problems

From @AmozPay : I don’t quite see how describing new features suits with describing problems and their solutions though. It is in my eyes two very different approaches

It is indeed disturbing at first, but you can in fact describe features as problems like in @MikeMilkyway 's example.

Problem: Users cannot login using Solana
Solution: Add Solana wallet integration
Problem: Developers cannot easily run Docker containers in VMs
Solution: ...

PRs look like issues

From @odesenfans : My only gripe with the idea is that it names everything a problem, which can make PRs look a bit too much like issues.

I forked the ZeroMQ text to adapt it in consequence with the use of other keywords to avoid the redundant “Problem:” keyword everywhere. We can use something more explicit for release notes such as “Feature/Fix/Chore”.

Not every commit deserves a description

From @BjrInt : I think it’s not a bad idea per say, but I don’t think every commit deserves a description. Even in large distributed codebase you would find single line commit, and refer to the code for extra information.

From @odesenfans : IMO “Fix: fixed a typo in the doc” of “Fix: made method of X async” can be self-standing commits and don’t require a 3-page explanation below.

I added an example where the description does not make sense since it is just a duplication of the title of the commit to make clear that this is okay.

Devs will have to cleanup they history

From @MHHukiewitz : Applying this new style to commits would require that devs, from time to time, squash their commits and update the commit messages

Yep. IMO commits should represent a clearly defined unit of work. What @odesenfans and @hoh usually do while working is to make nonsensical commits, and once they are getting somewhere they just squash everything in one or a few commits. If the commits get too big or are unrelated, they split them in multiple commits/PRs.

Using git rebase -i HEAD~10 (10 last commits here) helps a lot to cleanup history.

1 Like