docs/contributing/commit-discipline.md
We follow the Git project's own commit discipline practice of "Each commit is a minimal coherent idea". This discipline takes a bit of work, but it makes it much easier for code reviewers to spot bugs, and makes the commit history a much more useful resource for developers trying to understand why the code works the way it does, which also helps a lot in preventing bugs.
Use git rebase -i as much as you need to shape your commit structure. See the
Git guide for useful resources on mastering Git.
Tip: Check out how to use git log -p to study the commit
history of the project.
Whenever possible, find chunks of complexity that you can separate from the rest of the project.
Zulip expects you to structure the commits in your pull requests to form
a clean history before we will merge them. It's best to write your
commits following these guidelines in the first place, but if you don't,
you can always fix your history using git rebase -i (more on that
here).
Never mix multiple changes together in a single commit, but it's great to include several related changes, each in their own commit, in a single pull request. If you notice an issue that is only somewhat related to what you were working on, but you feel that it's too minor to create a dedicated pull request, feel free to append it as an additional commit in the pull request for your main project (that commit should have a clear explanation of the bug in its commit message). This way, the bug gets fixed, but this independent change is highlighted for reviewers. Or just create a dedicated pull request for it. Whatever you do, don't squash unrelated changes together in a single commit; the reviewer will ask you to split the changes out into their own commits.
It can take some practice to get used to writing your commits with a clean history so that you don't spend much time doing interactive rebases. For example, often you'll start adding a feature, and discover you need to do a refactoring partway through writing the feature. When that happens, we recommend you stash your partial feature, do the refactoring, commit it, and then unstash and finish implementing your feature.
For additional guidance on how to structure your commits (and why it matters!), check out GitHub's excellent blog post.
Commit messages have two parts:
In Zulip, commit summaries have a two-part structure:
Here is an example of a good commit message:
tests: Remove ignored realm_str parameter from message send test.
In commit 8181ec4, we removed the
realm_stras a parameter forsend_message_backend. This removes a missed test that included this as a parameter for that endpoint/function.
The commit message is a key piece of how you communicate with reviewers and future contributors, and is no less important than the code you write. This section provides detailed guidance on how to write an excellent commit message.
Tip: You can set up Zulip's Git pre-commit hook to automatically catch common commit message mistakes.
The first part of the commit summary should only be 1-2 lower-case
words, followed by a :, describing what the part of the product the
commit changes. These prefixes are essential for maintainers to
efficiently skim commits when doing release management or
investigating regressions.
Common examples include: settings, message feed, compose, left sidebar, right sidebar, recent (for Recent conversations), search, markdown, tooltips, popovers, drafts, integrations, email, docs, help, and api docs.
When it's possible to do so concisely, it's helpful to be a little more
specific, e.g., emoji, spoilers, polls. However, a simple settings: is better
than a lengthy description of a specific setting.
If your commit doesn't cleanly map to a part of the product, you might
use something like "css" for CSS-only changes, or the name of the file
or technical subsystem principally being modified (not the full path,
so realm_icon, not zerver/lib/realm_icon.py).
There is no need to be creative here! If one of the examples above fits your commit, use it. Consistency makes it easier for others to scan commit messages to find what they need.
Additional tips:
This is a complete sentence that briefly summarizes your changes. There are a few rules to keep in mind:
provision: Improve performance of installing npm.channel: Discard all HTTP responses while reloading.integrations: Add GitLab integration.typeahead: Rename compare_by_popularity() for clarity.typeahead: Convert to ES6 module.tests: Compile Handlebars templates with source maps.blueslip: Add feature to time common operations.gather_subscriptions: Fix exception handling bad input.channel_settings: Fix save/discard widget on narrow screens.The body of the commit message should explain why and how the change was made. Like a good code comment, it should provide context and motivation that will help both a reviewer now, and a developer looking at your changes a year later, understand the motivation behind your decisions.
Many decisions may be documented in multiple places (for example, both in a commit message and a code comment). The general rules of thumb are:
As an example, if you have a question that you expect to be resolved during the review process, put it in a PR comment attached to a relevant part of the changes. When the question is resolved, remember to update code comments and/or the commit description to document the reasoning behind the decisions.
There are some cases when the best approach is improving the code or commit structure, not writing up details in a comment or a commit message. For example:
When you fix a GitHub issue, mark that you have fixed the issue in
your commit
message
so that the issue is automatically closed when your code is merged,
and the commit has a permanent reference to the issue(s) that it
resolves. Zulip's preferred style for this is to have the final
paragraph of the commit message read, e.g., Fixes #123..
Note: Avoid using a phrase like Partially fixes #1234., as
GitHub's regular expressions ignore the "partially" and close the
issue. Fixes part of #1234. is a good alternative.
The commit summary and description should, taken together, explain to another Zulip developer (who may not be deeply familiar with the specific files/subsystems you're changing) why this commit improves the project. This means explaining both what it accomplishes, and why it won't break things one might worry about it breaking.
False as part of checking your work -- what you're doing is
providing them a chain of reasoning that they can verify.You can
credit
co-authors on a commit by adding a Co-authored-by: line after a blank line at
the end of your commit message:
Co-authored-by: Greg Price <[email protected]>
You should always give credit where credit is due. See our guide on continuing unfinished work for step-by-step guidance on continuing work someone else has started.
You can also add other notes, such as Reported-by:, Debugged-by:, or
Suggested-by:, but we don't typically do so.
@-mentions don't belong in commit messages (and won't notify the user you mentioned). If you want to send someone a notification about a change, @-mention them in the PR thread.
There are a few specific formatting guidelines to keep in mind:
git log in a normal terminal. (It's OK for links
to be longer -- ignore gitlint when it complains about them.)Tip: You may find it helpful to configure Git to use your preferred editor
using the EDITOR environment variable or git config --global core.editor,
and configure the editor to automatically wrap text to 70 or fewer columns per
line (all text editors support this).