Re: IGT contributions and reviews

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Qua, 2016-10-19 às 09:50 +0200, Daniel Vetter escreveu:
> On Tue, Oct 18, 2016 at 07:33:10PM +0300, Jani Nikula wrote:
> > 
> > On Tue, 18 Oct 2016, Petri Latvala <petri.latvala@intel.com> wrote:
> > > 
> > > The current contributing docs for IGT state:
> > > 
> > > <<
> > >   There is no formal review requirement and regular contributors
> > > with
> > >   commit access can push patches right after submitting them to
> > > the
> > >   mailing lists. But invasive changes, new helper libraries and
> > >   contributions from newcomers should go through a proper review
> > > to
> > >   ensure overall consistency in the codebase.
> > > > 
> > > > > 
> > > > > 
> > > 
> > > 
> > > While not requiring reviews or acks has definitely increased the
> > > speed of development, I feel the time for slowing down a bit has
> > > come.
> > 
> > Agreed. (Though a more rigorous review requirement doesn't
> > necessarily
> > slow things down in the big picture.)
> > 
> > > 
> > > At the very least I would like to see all commits have a visit to
> > > the
> > > mailing list before pushing, as the current docs already ask for.
> > > The
> > > "right after" part would be changed to a $period of quarantine,
> > > maybe
> > > 24 hours?
> > 
> > Sounds good to me.
> 
> We've already had this, and people stopped bothering. What will be
> different this time around? I feel a bit like we do need to be a bit
> more
> formal here, to really make this stick ...

I think the problem is that if only one single person doesn't follow
the rules, a few others will notice and start thinking: "If this guy
doesn't need to follow the rules, why do I need to? Why is Daniel
asking me to submit patches to the mailing list if that guy doesn't
seem to need to follow the same rule? Is this some sort of double-
standard?". Then person #2 stops following the rule, which makes more
people realize the same thing, which makes person #3 and #4 stop, etc.
So please, whatever rule we decide, let's make sure *everybody has to
follow it*, no exception for special cases, no allowing-people-to-get-
away-with-it. No double standards. </rant>

As a consequence of what I said, we do need to think: what if someone
doesn't follow the rule we decide? What will we *actually* do? Will it
really work if all we do is to politely ask them on IRC? Maybe the lack
of consequences is that degraded our previous rule?


> 
> Also, who's going to be the annoying maintainer who reminds everyone
> every
> time they break the rules? It'll take some serious effort here to get
> folks off their well-trodded paths onto a new one I think.
> -Daniel
> 
> > 
> > > 
> > > As for requiring reviews or acks before pushing, how do the
> > > developers
> > > at large feel about that? Different rules for different parts of
> > > IGT?
> > > (Benchmarks, tools, tests, CI test sets, lib....)
> > 
> > I think there are two big buckets here:
> > 
> > * Tests in BAT and the BAT set list. I think we need r-b/ack on the
> >   mailing list on these changes before pushing. (In the long run,
> > I'd
> >   like to have these go through a CI run with everything else
> > unchanged
> >   too.)
> > 
> > * Everything else. Other tests and tools. I'd be happy with
> > requiring
> >   the patches are sent to the list, and either receiving r-b/ack or
> > 24
> >   hrs during weekdays without negative feedback.
> > 
> > > 
> > > The goal with this discussion is to reach a suitable tradeoff
> > > between
> > > stability from CI point of view and fruitful use of programmer
> > > time.
> > 
> > Thanks for starting the discussion.
> > 
> > BR,
> > Jani.
> > 
> > 
> > -- 
> > Jani Nikula, Intel Open Source Technology Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx