61 views
# AEP-010: git commit style #10 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 ](http://wiki.zeromq.org/docs:contributing#toc3) 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. 3. A blank line. 4. 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.