Re: CF3+4 (was Re: Parallel query execution)

On Sunday, January 20, 2013, Simon Riggs wrote:

> On 20 January 2013 18:42, Robert Haas <robertmhaas(at)gmail(dot)com<javascript:;>>
> wrote:
> > On Sat, Jan 19, 2013 at 5:21 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com<javascript:;>>
> wrote:
> >> Sometime this type of high-level summary review does happen, at the
> senior
> >> person's whim, but is not a formal part of the commit fest process.
> >>
> >> What I don't know is how much work it takes for one of those senior
> people
> >> to make one of those summary judgments, compared to how much it takes
> for
> >> them to just do an entire review from scratch.
> >
> > IME, making such summary judgements can often be done in a few
> > minutes, but convincing that the patch submitter that you haven't
> > created the objection purely as an obstacle to progress is the work of
> > a lifetime. We could perhaps do better at avoiding perverse
> > incentives, there.
>

As a junior reviewer, I'd like to know if my main task should be to decide
between 1) writing a review convincing you or Tom that your judgement is
hasty, or 2) to convince the author that your judgement is correct. That
would provide me with some direction, rather than just having me pondering
whether a certain variable name ought to have one more or one fewer
underscores in it. On the other hand if a summary judgement is that the
patch is fundamentally sound but needs some line-by-line code-lawyering, or
some performance testing, or documentation/usability testing, or needs to
ponder the implications to part XYZ of the code base (which neither I nor
the other have even heard of before), that would also be good to know.

My desire is not for you to tell me what the outcome of the review should
be, but rather what the focus of it should be.

As it is now, when I see a patch that needs review but has no comments, I
don't know if that is because no senior developer has read it, or because
they have read it but didn't think it needed comment. The wiki page does
list points to consider when reviewing a submission, but not all points are
equally relevant to all patches--and it is not always obvious to me which
are more relevant.

> (Without meaning to paraphrase you in any negative way...)
>
> Judgements made in a few minutes are very frequently wrong, and it
> takes a lot of time to convince the person making snap decisions that
> they should revise their thinking in light of new or correct
> information.

This is true, but I'd like to know up front that convincing them to revise
their thinking is the main thing I need to do during the review process.
Restating and clarifying the submitter's arguments, perhaps with the
addition of some experimental evidence, is one part where I think I can
contribute, provided I know that that is what I need to be doing. Having
them reserve their opinion until after it is marked "ready for commiter"
doesn't make it easier to change their mind, I don't think. As a reviewer
I can't help address their specific concerns, if I don't know what those
were.

Cheers,

Jeff