On 9/5/08, Marko Kreen <markokr@gmail.com> wrote:
> The list is correct but too verbose. And it does not attack the core
> of the problem. I think the problem is not:
>
> What can/should I do?
>
> but instead:
>
> Can I take the responsibility?
To clarify it - that was the problem I faced last commitfest.
Basically, I'm not a newbie, but I'm not deeply familiar with most of the
components in Postgres. I'm not afraid to patch any part of code,
because I know somebody who *is* familiar with the part will later
review it. But if I'm supposed to answer "Is this patch commitable?"
then this is too much for me.
But when I convinced myself that my only task is to decrease the amount
problems a patch has, so that committers job will be easier, then I felt
at ease to take stab on several of them.
So I suppose this works for other too and maybe this is worth accepting
as official policy - the reviewers job is not to guarantee some level
of quality, but just to report any problems they are able to find,
so that committer's job will be easier and he can concentrate on the
in-depth review.
All this assumes you want relatively nobodies doing the reviews. If you
want guaranteed quality, then this will scare away lightweights or make
them look only one aspect of the patch.
This leaves the heavyweights, but as you know, they are busy..
> Lets say reviewer would like look on coding style or performance.
> ATM it seems to him he well be now fully responsible for that aspect.
>
> I think we have better results and more relaxed atmospere if we
> use following task description for reviewers:
>
> The committer will do in-depth review. You task as a reviewer
> is to take off load from committers by catching simple problems.
> Your task is done if you think the patch is ready for in-depth
> review from committer.
>
> Note1 - Yes, the trick is to emphasize that all responsibility
> lies on committer.
>
> Note2 - detailed lists of areas to look at and reviewer types are not
> useful as each patch is different and each revier is different.
> Long lists just confuse people. The simpler the better.
>
> The main thing is to make easy for reviewer to take the first look.
--
marko
