RFC: xserver commit process changes

[Keith Packard]

Peter Hutterer <peter.hutterer at who-t.net> writes:

> This has worked reasonably well since we started for server 1.8. If 
> nothing else, it has made git master a lot more reliable. However, Keith 
> is the bottleneck to get anything into master. The delay to get a pull 
> request merged has been quite random, depending on Keith's other 
> workload. I'm struggling with this a lot at times. Bugs cannot be closed 
> even though the work is done, local branches cannot be merged and 
> finalised. Apparently others face similar issues.

Yeah-- my delay is somewhere between an hour and a week, depending on
other things going on. And, the process for merging patches is pretty
formalized now that we've done this a lot. It's not entirely mechanical
though, the key is to have some idea whether a patch has 'sufficient'
'credible' review, and whether there are any outstanding questions about
it.

> * leave the current window of 3/2/1-ish months for the different devel 
> stages
> * leave the requirement for a reviewed-by
> * one RM, calling the shots for when releases are made and generally 
> being the reviewer of last resort and arbiter where needed

> * 3-5 people with commit access during the devel and general bugfix 
> windows. They scoop up pull requests and commit them, if the patches 
> have rev-by tags [3]

It's a more subtle process than you might imagine; I really do read
every comment made during review and try to get a sense of whether the
code being merged is really ready.

This is really trying to do two things:

 1) Let people learn how to review code, both by watching others and by
    trying their own hand at it. When someone starts reviewing code that
    they haven't seen before, the first several review messages often
    catch C semantic and/or syntactic issues, but it isn't until the
    reviewer has seen a bunch of the code that they start to catch more
    subtle issues.

    Yes, I really do read all of the review comments that float past in
    email, and keep track of both who writes and who reads all of the code.

 2) Ensure that discussion about changes has come to rough consensus
    before changes are merged to master. Often times a patch will
    appear, seem reasonable to most people and get reviewed rapidly only
    to have someone pop up with valuable questions.

> * 2 people during the last bugfix window (emergency fixes only). The 
> second person as backup to the RM to make sure we don't see delays.

I'd say the tree should probably get locked to the RM for the last week
or so just to make sure things don't change unexpectedly.

> Volunteers for committers are welcome. Though note that this is not to 
> get your own patches in quickly, you'll also have to scoop up and review 
> other patches. (fwiw, I volunteer)

In fact, as a general rule committers to master shouldn't be pushing
major chunks of their own code to ensure that at least two people are
thinking closely about the development process. This will help me a
bunch as I've often felt a bit uncomfortable merging my own development
work into the tree, even with reviewed-by lines attached.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20130228/3e0b9410/attachment.pgp>