[development] Splitting up patches

[Neil Drumm]

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 
work.

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).

Two examples:

* 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 
they:
   - 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 
patch posted.
   - 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 
inseparable.

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.

-- 
Neil Drumm
http://delocalizedham.com/