[development] Splitting up patches
drumm at delocalizedham.com
Thu Aug 24 23:05:20 UTC 2006
I've asked for many patches to be split up while I've been a core
committer. Recently this has hit a couple nerves, so I'll explain a bit.
Larger patches take more effort to review. There is more to test and
more to think about. There are more places for bugs to hide and
unintended changes to be hidden in.
My job is to get as many good patches into the next release of Drupal as
possible. My job is easier if patches are easier to review for myself
and other reviewers.
The different sections of a patch are usually of variable quality, some
may go in immediately with only spot testing and some need thinking. I
don't like rejecting good parts of patches because other parts need more
When you are making a complex patch, weigh the effort for yourself to
make smaller patches against the combined effort of reviewers (you
should have at least two, someone else and then a core committer).
* PHP notice cleaning, which I like to see happen. Every change needs to
be examined for correctness and potential for side effects. These
patches usually have:
- 3 to 10 individual changes
- a pithy description like "Remove some PHP notices"
- about half the changes are good and I'm not so sure about the others
- no one else has reviewed it (I've already handled the
ready-to-commits and criticals)
If it were separated into smaller patches I could get half of the thing
done and we could have specific discussion on the remainder.
* Giant API or functionality changes, which I also like to see happen.
These need to be somthing most people want and implemented well. Usually
- are big and hard to be sure if everything relevant is tested.
- have at least 50 follow-ups already, with less than one review per
- change one or two things as a side-effect of the main thing we
should be reviewing.
- always have bugs, some will be found after it is committed.
- have a some interrelated changes that are big, complex, and
If the side-effect changes happened separately those could be committed
faster and make the rest smaller and easier to review and test.
For those still reading this rant that is too long, I think a plumbing
analogy is appropriate. Think of the patch queue as a funnel with Dries
and myself at the small end and over 200 patches in it at any given
time. The larger patches clog the funnel and prevent them from being
applied and sometimes block the path of other smaller patches. If
patches are smaller they can flow through the funnel at a more steady
and consistent rate, and get more things accomplished.
If I am mistaken about the separation of a patch, then do what you do
any other time I'm wrong (it happens)- provide a well-reasoned and
concise response. There are no hard rules here since it is a bit subjective.
More information about the development