New development model check-in.

[Tiago Vignatti]

Tiago Vignatti wrote:
> 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.

s/our process/write our process