How to get a patch into Rails
Posted by David September 15, 2007 @ 09:46 PM
For a while, the patch queue was getting badly out of hand. The flood of new entries was simply too great to be reasonably managed by a small group of people. Too much got stuff got stale and their creators got disillusioned, understandably so. But for a while now, we’ve been running with a new policy on patches, which seems to be working a lot better for those who’ve been following it.
But I’m sensing that a fair number of people are not aware of those changes in policy, so I thought best to bring them up again here.
Step 1: Raise the barriers of quality
Part of the reason that the original patch queue got out of hand was due to the large number of patches coming in that lacked essential qualities of a good patch. They were either missing a good rationale (why am I doing this? what’s the benefits?), good test cases, or didn’t update the relevant documentation. To apply such a patch meant that this work had to be shouldered by someone else, usually the guy who wanted to apply the patch.
Now the barriers of quality are more apparent. Your patch will simply not be considered for inclusion before it has all those elements. It’ll live with the “unverified” keyword until you or someone else that cares especially deeply about the patch (like someone else having the same problem) gets the quality up to par. Then the patch moves on to step 2.
Step 2: Get the community engaged to review your patch
The last step before your patch is ready to be put in the queue for inclusion is to get community support round up. We now require that three different people must review your patch, apply it, run the tests, read your documentation, and like what it does and how it’s implemented. When they do, they’ll make a comment on the ticket with a “+1”.
Get three such +1s and you can tag your patch with the “verified” keyword. That’ll make the patch appear in Report #12: Verified Patches, which is a bell telling the core team that you patch is baked and ready to be included (barring a final review).
The core team is trying to keep report #12 empty at all times. There shouldn’t be a big lag time between getting to “verified” and getting a final review of your patch, which will either send it back to unverified (because the implementation is deemed in need of work or because there’s some fundemental disagreement over whether or how this patch goes about its business) or it’ll be applied and available in edge.
So if you have a patch that you still care about sitting in the queue, dust it off, and put it through these two simple steps and you’ll be back on the road to glory. There are still no guarantees that your patch will receive immediate attention, but so far we’ve managed to keep report #12 moving very nicely. It’s all empty as of today!
The front page of http://dev.rubyonrails.org has been updated to reflect this policy.
One thing I’ve been unclear on – who can GIVE the +1s? Any three people, or three committers? There are times I see a good patch I’d be happy to try, but I don’t bother because I’m not sure I get a vote.
Everyone gets a vote. You just have to be enough into it that you can review the code on at least a superficial level. And you’re able to run tests etc.
I clearly interpret David’s post as to mean any three people:
We now require that three different people must review your patc
Some guidelines on how to sumbit a patch for those of us not familiar with Trac would rock too.
Those sort of things.
Thanks for the heads up David. The patch system was getting pretty unwieldly.
I know its kind of off topic, but I’m wondering… is there any sort of estimated date/timeline for Rails 2.0?
So basically anyone who isn’t one of your little IRC butt buddies doesn’t get to work on your pet project. How is this different exactly?
Is there anyway to shortcut the process for small patches? Seem like a tall order to expect 3 people to +1 a one line change.
You can usually find people to review your patches in #rails-contrib, especially for tiny ones.
@DHH: That’s great; thanks for clarifying. So those of us who’d like to contribute by reviewing can keep an eye on the unverified patches report.
@Sean: You could also get six people who don’t really care to give a +0.5… no, but there’s a separate report for tiny patches. So maybe some of us could spend some time verifying those. TinyPatch day, anyone?
@Pratik: Good point, and don’t forget the rails-core mailing list. If your patch is sitting around waiting for verifications, it’s OK to ping the list and ask for some patch love…
+1 to Sean/Jay
@Marc: No one seems to have given a date yet. Sounds like it’s still in the months not weeks time frame. Here’s an attempt at seeing if the “crowd” can figure it out (like guessing how many jelly beans are in the jar at the fair):
http://home.inklingmarkets.com/trades/new/23203
I think it should be better to stop using patches and start using a distributive version control system. I think it should simplify the process.
I am unsure about the “unverified” keyword. Where does it come it from? Or perhaps, the “verified” keyword – who can add that to a ticket, and based on what criterias?
The BBC are using rails here!! :
http://catalogue.bbc.co.uk/
It’s a searchable back catalogue!
This should be posted somewhere decent!
I agree with Matteo: David, you really should have a look at a DVCS, it completely changed my workflow when I switched from Subversion to Bazaar. And if you try Git, remember that it’s not quite the most user-friendly tool—there are alternatives.
Cheers, Daniel