New development model check-in.

[Tiago Vignatti]

Keith Packard wrote:
> On Tue, 24 Nov 2009 14:04:21 +1000, Peter Hutterer <peter.hutterer at who-t.net> 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.


                     Tiago