On Multiple Patches in Bugs
January 07, 2014 at 04:40 PM | categories: MozillaThere is a common practice at Mozilla for developing patches with multiple parts. Nothing wrong with that. In fact, I think it's a best practice:
- Smaller, self-contained patches are much easier to grok and review than larger patches.
- Smaller patches can land as soon as they are reviewed. Larger patches tend to linger and get bit rotted.
- Smaller patches contribute to a culture of being fast and nimble, not slow and lethargic. This helps with developer confidence, community contributions, etc.
There are some downsides to multiple, smaller patches:
- The bigger picture is harder to understand until all parts of a logical patch series are shared. (This can be alleviated through commit messages or reviewer notes documenting future intentions. And of course reviewers can delay review until they are comfortable.)
- There is more overhead to maintain the patches (rebasing, etc). IMO the solutions provided by Mercurial and Git are sufficient.
- The process overhead for dealing with multiple patches and/or bugs can be non-trivial. (I would like to think good tooling coupled with continuous revisiting of policy decisions is sufficient to counteract this.)
Anyway, the prevailing practice at Mozilla seems to be that multiple patches related to the same logical change are attached to the same bug. I would like to challenge the effectiveness of this practice.
Given:
- An individual commit to Firefox should be standalone and should not rely on future commits to unbust it (i.e. bisects to any commit should be safe).
- Bugzilla has no good mechanism to isolate review comments from multiple attachments on the same bug, making deciphering simultaneous reviews on multiple attachments difficult and frustrating. This leads to review comments inevitably falling through the cracks and the quality of code suffering.
- Reiterating the last point because it's important.
I therefore argue that attaching multiple reviews to a single Bugzilla bug is not a best practice and it should be avoided if possible. If that means filing separate bugs for each patch, so be it. That process can be automated. Tools like bzexport already do it. Alternatively (and even better IMO), we ditch Bugzilla's code review interface (Splinter) and integrate something like ReviewBoard instead. We limit Bugzilla to tracking, high-level discussion, and metadata aggregation. Code review happens elsewhere, without all the clutter and chaos that Bugzilla brings to the table.
Thoughts?