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

Поиск
Список
Период
Сортировка
От Jeff Janes
Тема Re: CF3+4 (was Re: Parallel query execution)
Дата
Msg-id CAMkU=1xxjf=Q5n+NQq9Z4U74N4aqnoWUkti9tvYZbZcL_qFTtw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: CF3+4 (was Re: Parallel query execution)  (Simon Riggs <simon@2ndQuadrant.com>)
Ответы Re: CF3+4 (was Re: Parallel query execution)  (Robert Haas <robertmhaas@gmail.com>)
Re: CF3+4 (was Re: Parallel query execution)  (Heikki Linnakangas <hlinnakangas@vmware.com>)
Список pgsql-hackers
On Sunday, January 20, 2013, Simon Riggs wrote:
On 20 January 2013 18:42, Robert Haas <robertmhaas@gmail.com> wrote:
> On Sat, Jan 19, 2013 at 5:21 PM, Jeff Janes <jeff.janes@gmail.com> 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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Robert Haas
Дата:
Сообщение: Re: Removing PD_ALL_VISIBLE
Следующее
От: Peter Geoghegan
Дата:
Сообщение: Re: Doc patch making firm recommendation for setting the value of commit_delay