Watzmann.Blog Varying amounts of fiber

How to ensure your patches will face slow and protracted review

11 May 2012

I’ve been looking at a few fairly long patch series lately, and there are a some things people do that make reviewing them harder (and slower) than it needs to be. Not aimed at anyone in particular, and in no specific order:

* Send in your work of the last three months as one big patch; there's
no reason for anybody else to try and understand every detail you had
to wrestle with.

* Send in your work of the last three months as 100 patches; be honest
with yourself, you're just trying to intimidate possible reviewers.

* Keep fixes to bugs introduced by one of your patches in a separate
patch; to fully appreciate what you've done, reviewers should be able
to trace your entire thought process, including the mistakes you made
on the way.

* Reformat everything to different indentation in patch 12/30; even if
the reviewer thinks that patches should be combined or reordered, make
it clear that you disagree.

* Throw in fixes to unrelated problems; you've worked hard on this
patch series, and you now understand so many more shortcomings of the
code base.

* Change ancillary details like license and copyright headers; the
project might have different ideas how that is handled, but since you
did great work for them, they should change their ways out of gratitude

A good patch series should be focussed, short, and to the point; in a way, it should be a story that leads up to a punchline. Such a patch series allows the reviewer to stay focussed on the topic of the series, and makes it easy for them to follow what you are trying to achieve, and not how you got there. The hallmarks of a good patch series are:

* The series serves a single purpose; fixes to problems in the existing
  code should either come first in the series, or, even better, should
  be submitted separately
* The series is as small as possible; reducing the number of patches in
  it would violate one of the other points
* Each patch in the series is a small focussed step towards the overall
  goal of the series
* Applying the series patch-for-patch yields a functional code base
  after each step; in particular, all tests will pass after each step

Creative Commons License Watzmann.Blog by David Lutterkort is licensed under a Creative Commons Attribution-Share Alike 3.0 United States License.

Generated with Jekyll