On Tue, 24 Nov 2009 14:04:21 +1000, Peter Hutterer <[email protected]> wrote:
> I thought it was you as CC but that's not always the case, in fact I noticed > that some patches sent for general review get applied directly to master. > So in preparing a patchset with several patches, some of them are already > there by the time I'm ready to send a pull request. Sorry, I haven't been consistent with this, and I really should be. I was trying to be "nice" to people who had posted patches that seemed to be reasonable. I'll try to check myself and let people tell me that they think things are ready by explicitly Cc:'ing me. > In two concrete cases: > - my pull request was reduced to a single patch. That feels a bit meager to > send a pull request for. Not really; if the patch has been reviewed and happens to be sitting in your repository, a pull is the same as a patch for me (I review in gitk instead of in the email; not a huge difference). > - a patch was applied to master and then a day later reviews come in that > require changes to this patch. Pull requests tend to be a bit delayed, so > that can be caught by those. Right, that's my fault for not waiting for people to tell me that things are ready by having a Reviewed-by: line *and* an explicit Cc:. > Add on on top of that your habit of cherry-picking patches from a pull request > which makes it harder to have a fast-forward branch. I think I only did this once, and you're right, it was a bad idea. Won't happen again. Pulls are all-or-nothing, no cherry-picking allowed. > - send patches to list for review, only merge into master if asked for Perhaps instead of using just the Cc: trick, you could just stick a comment in the patch mail letting me know that this patch has seen sufficient review and is ready to be merged to master. That should avoid accidents with addressing. > - send pull requests Make sure every patch in the pull request has a Reviewed-by: line. > - patches in pull requests that aren't suitable should get reverted with an > explanation. Pull requests are all-or-nothing; I don't want to break people's trees. > I think this might suit Michel a bit better as well, since he could replace > 'git push origin master' with 'git push fdo master' and the occasional pull > request. He doesn't need to wait for you applying patches, he needs to keep > track of public review himself which I think isn't largely different to the > old "push to master" model. That's what I'd like to see for any 'subsystem' maintainer -- push out a patch series to their public tree and, when ready, send me a pull request. If I keep pull requests simple merges, then their history will be preserved intact and 'merging' back will be trivial. > Of course, that doesn't change the delay in getting patches to master. We've got two outstanding issues which aren't on master; the fb bug that breaks compositing from some source images and the exa series. For the fb thing, I want to see someone try and explain to me why copying data From one pixmap to another 'helps' in any way. For exa, I need additional review; I don't have any way of testing that code at this point. So, changes I will make: 1) Wait for people to explicitly ask for patches to be merged. 2) Pulls are pure merges, no fancy tricks. Keep your suggestions coming; the goal is to make things 'better' for everyone, not to slow things down or keep people from getting useful work done. Please feel free to publish your own trees with stuff you'd like people to see; the more testing we get on each patch, the better the final product will be. -- [email protected]
pgpw5LbkrwRNJ.pgp
Description: PGP signature
_______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
