osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[dev][all] note to non-core reviewers in all projects


Hello everyone,

A recent spate of +1 reviews with no comments on patches has put me into
grumpy-old-man mode.

A +1 with no comments is completely useless (unless you have a review on
a previous patch set with comments that have been addressed by the
author).  I already know you're a smart person (you figured out how to
sign the CLA and navigate gerrit -- lots of people can't or won't do
that), but all your non-comment +1 tells me is that you are in favor of
the patch.  That doesn't give me any information, because I already know
that the author is in favor of the patch, so that makes two of you out
of about 1,168 reviewers.  That's not exactly a groundswell of support.

When you post your +1, please leave a comment explaining why you
approve, or at least what in particular you looked at in the patch that
gave you a favorable impression.  This whole open source community thing
is a collaborative effort, so please collaborate!  You comment does not
have to be profound.  Even just saying that you checked that the release
note or docs on the patch rendered correctly in HTML is very helpful.

The same thing goes for leaving a -1 on a patch.  Don't just drop a -1
bomb with no explanation.  The kind of review that will put you on track
for becoming core in a project is what johnthetubaguy calls a
"thoughtful -1", that is, a negative review that clearly explains what
the problem is and points the author in a good direction to fix it.

That's all I have to say.  I now return to my normal sunny disposition.

cheers,
brian