[pulseaudio-discuss] Rethinking how we do reviews

[Tanu Kaskinen]

On Mon, 2016-04-04 at 13:35 +0530, Arun Raghavan wrote:
> On 4 April 2016 at 13:01, David Henningsson <diwic at ubuntu.com> wrote:
> > 
> > 
> > 
> > On 2016-04-04 04:51, Arun Raghavan wrote:
> > > 
> > > 
> > > On Sat, 2016-04-02 at 17:19 +0300, Tanu Kaskinen wrote:
> > > > 
> > > > 
> > > > On Thu, 2016-03-31 at 10:55 +0530, Arun Raghavan wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > We have two opposing axes to optimise along. The first is code quality,
> > > > > as you point out. The other is developer motivation.
> > > > > 
> > > > > My feeling is that being able to work on things and push them out
> > > > > without a long wait would help us feel more productive and motivated to
> > > > > work on all aspects of the project.
> > > > > 
> > > > > With the current system, I think we're spending more and more time
> > > > > reviewing things and waiting for reviews.
> > > > > 
> > > > > Having the ability to work on things and push them out provides a much
> > > > > shorter path between "this is something I'd like to do" and "this thing
> > > > > is done". This allows for a positive feedback loop, and I think that
> > > > > will bolster our motivation to perform reviews, bug triaging, and
> > > > > everything else.
> > > > That sounds like a plausible theory, although I don't personally feel
> > > > demotivated by the large amount of time it takes for my own patches to
> > > > get merged. If you really think that your review bandwidth increases if
> > > > your own patches go through faster, it seems to me that I could
> > > > prioritize reviewing your patches over others, and that way get more
> > > > reviews from you. What do you think?
> > > I think that a this would help, but I fear that this does not do
> > > enough. My preference is still the relaxed (but still existent) review
> > > process that I outlined before, so that we have /some/ hope of catching
> > > up on the patch and bug backlog over time.
> > > 
> > > Perhaps we can give it a try what I'd proposed for a couple of release
> > > cycles and evaluate how things stand?
> > > 
> > > 
> > For me, the demotivator has been when we disagree on things, if someone
> > wants my patch written in another way: I can then choose either to rewrite
> > my code to someone else's liking even though I like it better the way it is
> > (which is demotivating), or I can choose to respond back which can lead to
> > long discussions, which is in itself demotivating and time consuming.
> > I don't know if there exists a good solution to that problem, but I've been
> > thinking along the lines of having areas of responsibility, where one is
> > effectively dictator for some pieces of the code, and someone else is
> > dictator for other pieces, etc.
> I think there are ways to short-circuit long discussions, such as
> setting aside some time for real-time discussion on IRC.
> 
> And perhaps it does make sense to take on more autonomy for certain
> areas if that helps us work better. Note that my review proposal does
> overlap with your suggestion in that non-external-facing changes can
> be done more autonomously.

I'm not sure David's proposal overlaps with your proposal. David didn't
explicitly say whether these dictators themselves should still have
their own patches in their own area reviewed. I understood David's
proposal only as a tool to make it easier to decide things when there
are disagreements, so it wouldn't imply a right to skip reviews.

I quite like the general idea of having a dictator to resolve
disagreements, but it's not obvious how the code base could best be
divided. I don't myself have any particular desire to be the dictator
for any part of the code, but I don't have a problem with taking that
role either if it's given to me.

> > As for Tanu's suggestion (to prioritise Arun and see if that helps) vs
> > Arun's (every committer is free to push their own stuff), I prefer
> > evaluating Tanu's, as I see a risk with Arun's proposal that committers will
> > spend more time on their own stuff rather than reviewing, if the former is
> > what gives that positive feedback loop.
> To me, the latter gives a stronger positive feedback loop than the
> former, because you're still waiting for a few days at least for each
> patch set.

Does it really demotivate you if your patches take a few days to get
reviewed? I can understand that it's frustrating if reviews take weeks
or even months, but I have trouble imagining a few days of review delay
being a problem.

You didn't directly address my earlier point of reviews saving time. I
hesitate to accept your proposal because I believe that reviews save
time overall (our time and our users' time), since there are fewer bugs
to investigate and fix later. Do you believe differently?

> Committers spending more time on their own stuff is something that can
> just as well happen now

It doesn't make sense to spend more time on your own stuff, though, if
you produce patches faster than they get reviewed.

> -- nobody has to do reviews, we all choose to
> do them. I see the chances of a negative impact on review bandwidth
> being very low, and the changes of having a positive impact (from
> being more motivated) much higher.

-- 
Tanu