[Qemu-devel] Re: commit rules for common git tree

[Michael S. Tsirkin]

On Sun, Dec 27, 2009 at 11:23:26PM +0100, Aurelien Jarno wrote:
> On Sun, Dec 27, 2009 at 11:50:23PM +0200, Michael S. Tsirkin wrote:
> > On Sun, Dec 27, 2009 at 09:34:53PM +0100, Aurelien Jarno wrote:
> > > On Sun, Dec 27, 2009 at 01:37:32PM +0200, Michael S. Tsirkin wrote:
> > > > I'd like to discuss two questions related to changes that
> > > > are committed to the shared tree.
> > > > 1. A lot of patches are committed without being posted
> > > >    to the list first, thus they go in without review.
> > > >    Why is this good? Can this be addressed?
> > > 
> > > Most of the patches I commit without posting them first to the list are
> > > to fix bugs on non i386 targets, as they are broken too often by people
> > > who don't care about them.
> > 
> > I hope don't care is not really the approach any maintainers have.
> > Don't care enough to test them is probably what really happens :)
> > So IMO this is largely a problem of education and documentation.
> > E.g. on linux kernel I have never tested anything on alpha but
> > I use read_barrier_depends which is an empty macro except on alpha,
> > because there's excellent documentation that teaches when to use this
> > API.
> 
> It's not about maintainers, but people sending patches. Maintainers
> can't review all the patches in small details. People writing patches,
> should think about the impact it can have on linux-user or non-i386
> system target,

Better documentation is needed for this.  As it is absent, maintainers
should let people know when issues are discovered, and then controbutors
will learn.  Fixing bugs behind everyone's back will not do this.

> and also do some tests.
> > > I don't like leaving the tree broken too long so I prefer to fix that
> > > directly.
> > 
> > Another obvious option would be to have your own public tree.
> > You can then merge master when master is in a good shape.
> 
> It's not about me, it breaks the development for a lot of people.
> People are required to send their patches against HEAD. How to do if
> HEAD does not build or does not work?

This is not CVS. People could work against your tree, it would
get merged in master eventually.

> It will cost less time if one people fix the bug, than if 10 people
> have to workaround a broken tree.

Fix can introduce another bug. And so on ad infinitum.

For each given bug, there are n people for whom this is a show stopper.
So anything labeled bugfix should go in without review?
This is why we have distributed revision control: post a patch, people
can apply it in their trees and keep working. Or publish a tree and
people will work against it.  Just noting "this patch is on top of patch
X" in mail is all that is needed.

> > > If people prefer, I can simply revert the broken patch or
> > > series, even if sometimes it means reverting a series of 10 or more
> > > patches.
> > 
> > I agree an obvious push of a bigfix/revert might be a good idea.  Is
> > this really common? Also if you do so, could you mail the list after the
> > fact please?  This will serve to educate people about non i386 targets
> > by the way.
> 
> Ok, I will proceed this way, mailing the author and the list. I am
> still convinced it's a waste of time, but as long it is not my time,
> it's fine.
> 
> > Ideally problems would be caught *before* push to shared tree, revert
> > really should be last resort thing, right? Also, e.g. for PCI patches, I
> > always let non-obvious patches stay on my public tree for a while, it
> > would be very nice to test it before it is merged.
> 
> Not everyone has time to check trees from other people, unless we
> seriously limit the amount of patches.
> > > > 2. When a change is committed to the tree, often no notification is sent
> > > >    to the author.
> > > >    Why is it a good idea to ask everyone to subscribe to qemu commits
> > > >    list as well? Can 'applied thanks' mail be sent to patch authors?
> > > > 
> > > 
> > > It something that does not really goes in my workflow, as I most often
> > > commit a lot of patches in my local tree, and then after testing push
> > > (some of) them.
> > 
> > So why not mail people when you commit patches locally?
> > Everyone knows then that
> > - you don't see a problem with patch X
> > - if there's breakage they should let you know ASAP to avoid broken master
> > - they don't need to repost
> > 
> 
> When I merge a patch locally, it doesn't mean:
> - I will review it (sometimes I don't have time)
> - I consider the patch is fine
> 
> > > The best would be to get the qemu commits list working again.
> > 
> > As others pointed out, this is not friendly e.g. for reviewers.
> > We need better communication IMO, not better automation.
> > 
> 
> I am fine sending "applied" mails, but I will most probably review less
> patches. Please don't complain.

Why should I complain? The winner takes it all ... ^?^?^?^?^?^?^?^?^?^?^?^?^?^?
(sorry got carried away)

> -- 
> Aurelien Jarno                                GPG: 1024D/F1BCDB73
> address@hidden                 http://www.aurel32.net