On Thu, Feb 28, 2013 at 01:58:15AM -0800, Keith Packard wrote: > Peter Hutterer <[email protected]> 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.
Unfortunately, the delay is on top of the review delay. There's been at least two cases in the last cycle where a patch was lingering for weeks, unreviewed, and only got attention when I sent a pull request for unreviewed patches. e.g. "dix: don't allow disabling XTest devices", sent to the list on 11 Oct, sent as part of the pull request on Oct 25. That did get a review within a day then. (and I only picked this example because it's the first one that popped up in my mailbox) > > * 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. Both of these are welcome and nothing changes in that regard. I don't think that it needs to all fall on you though, there are others that can do the same as well and we should share the review burden much more. > > * 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. Yeah, that seems like a sensible plan. However, having a second person as a backup is a useful thing to avoid the bus factor. > > 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. Agreed, there have been plenty of times where I wasn't sure what I was cooking up. However, IMO there is still a need for some patches to go in faster than others. Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
