Tiago Vignatti wrote: > Keith Packard wrote: >> 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. >> > > > We'll be using that kernelnewbies patch submission process (state on > wiki) as a base for ours? Anyway, we should our process on the stone > somewhere.
s/our process/write our process _______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
