This page is primarily intended for Mercurial's developers.
One Change Per Patch
Why and how to make one change per patch.
1. What does "one change per patch" mean?
Mercurial's development process insists on "one change per patch" and code that doesn't follow this rule routinely gets rejected. This page tries to explain what "a change" is and why this rule exists.
One possible definition is: "a change is the smallest reasonable design increment." The ideal patch is "trivial and obviously correct". But it may be easier to define in terms of what is not a single change. Here are some examples:
- fixing a bug AND moving a function to a more logical location
- fixing a bug AND fixing an unrelated typo
- fixing a bug AND reformatting it for readability
Quite often, fixing a bug or adding a feature will itself involve multiple logical steps, for instance first factoring out a helper function, then rewriting part of that helper function.
In general, if your description of your change uses 'and' or includes a bullet list of changes... you've probably got more than one change. On the other hand, if you're doing some large but highly repetitive change (eg "change all uses of repo.foo to repo.bar"), then that's an acceptable single change.
2. Why so strict?
The short answer is: it's good engineering practice.
The traditional software development approach is "code, test, code, test, code, test, commit", where the final commit contains a large number of different changes that achieve some larger overall result. This is really unfortunate from an engineering perspective because it's not possible to isolate the history of the internal steps from each other. In the jargon of the scientific method, your change is an experiment on multiple independent variables, and just like such an experiment, it's hard to draw conclusions when the experiment fails.
Mercurial's strategy is instead "code, test, commit, code, test, commit", with each step being a clearly-delineated step towards our goal. This has various benefits:
Simple changes are much easier to review, because the reviewer doesn't need to independently correlate each line of change with a different piece of the commit description, or assign it to an implicit goal like "tidy things up as we go". A simple change is often "obviously correct" and can be reviewed with very little thought. Five simple changes rolled together are often impenetrable and often take much more than five times as long to properly review.
If the nth step of a series of changes doesn't pass review, the reviewer's time reading patches 1 through n-1 hasn't been wasted and won't have to be repeated on the next iteration. Those patches can be accepted, and the process has now made forward progress.
2.3. Bisectability and regression testing
If some part of a large commit introduces a problem that gets discovered later, it is much harder to isolate it from other changes. By contrast, with single-change commits, systematic search via bisect can rapidly pinpoint problems.
The following changes should never be combined:
- code movement
- whitespace and formatting changes
- style changes
- unrelated typo fixes
- bug fixes
- new features
In general, tests should pass before and after every change.
Thus, if a change breaks existing tests, those tests should be updated together with a test.
If a change fixes a bug, a test for that bug should be introduced either after or together with the corresponding change. Do not introduce failing or incorrectly-passing tests before a change.
4. Some examples
4.1. Breaking a complex change into a number of steps
These changes were all introduced in a single session to optimize performance of a single code path:
As it turns out, the first patch introduced a performance regression in another workload that was trivially located by an end-user via bisect. Had these patches been combined, it would have been very difficult to locate.
4.2. Multiple simultaneous changes causing problems
e8d37b78acfb introduces a new feature with a memory leak
8d44b5a2974f fixes the memory leak, but makes unrelated cleanups that introduce a new bug
b767382a8675 fixes the new bug
5. This discipline is hard!
Yes, but it becomes easier with practice and good tools. Unfortunately, core Mercurial (and SCMs in general) aren't particularly well-suited to the iterative refinement of changes implied here. Several tools that are helpful here are:
MQ - manage and edit a stack of changes
RebaseExtension - move development changes forward in history (works with MQ)
Histedit - another tool for editing and refactoring recent history
record - interactively commit parts of files from outstanding changes
Also, we have an ongoing effort to integrate intelligent mutable history into our core:
6. See also