Re: [Qemu-devel] We need more reviewers/maintainers!!

[Anthony Liguori]

On 03/13/2012 09:38 AM, Avi Kivity wrote:
> On 03/13/2012 04:00 PM, Anthony Liguori wrote:
> > On 03/13/2012 08:40 AM, Avi Kivity wrote:
> > > On 03/12/2012 10:27 PM, Anthony Liguori wrote:
> > > > > I agree that more maintainers would be good, but we also need
> > > > > more people with commit rights.
> > > >
> > > > I disagree strongly.  Having multiple pushers makes things difficult
> > > > and encourages people to push without testing.  Part of what makes
> > > > pushing take longer than it should today is that my test cycle takes
> > > > at least 1-2 hours and it's not uncommon to have to go through 3-4
> > > > cycles of rebasing before being able to push.
> > >
> > > This really sucks.
> > >
> > > If testing was automated, we could have a staging branch where
> > > maintainers would push patches, they'd get tested automatically and then
> > > graduate to master.  The workflow would look something like
> > >
> > >     git fetch
> > >     git checkout staging
> > >     git rebase origin/staging
> > >     <apply patches, pull trees>
> > >     git push staging
> > >     <wait>
> > >     <staging gets merged into master autoamatically, or you get an email
> > > from the test system>
> >
> > The problem for me with this is that I test before I do a thorough
> > review.  I do a quick review, but not a line-by-line review.  So I
> > don't necessarily want to queue for push.
>
> Seems to me it's better to review before testing, no?

I typically do a high level review before queuing for testing, but I don't do a 
line-by-line review for coding style or minor issues.

The later must be done before committing no matter how many revisions are sent. 
 It's more time efficient to catch a functional problem without doing the 
line-by-line review.  Best case scenario is that the line-by-line review happens 
only once for a patch before it's committed.

> > > If testing cannot be automated, perhaps a lock around the tree would
> > > help.
> >
> > I think merging qemu-test into make check would help a lot.  If all
> > committers are running the same test suite before pushing, then this
> > problem would become less common.  It's livable now because most
> > committers commit infrequently.
> >
> > But if we added more committers, it would become pretty problematic.
>
> I'm not arguing either for or against that, just trying to make the
> commit process more efficient.

Yup, and appreciate the suggestions.

Regards,

Anthony Liguori