[dev][all] note to non-core reviewers in all projects
ed at leafe.com
Tue May 21 15:10:23 UTC 2019
On May 21, 2019, at 8:31 AM, Brian Rosmaita <rosmaita.fossdev at gmail.com> wrote:
> 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.
I have to disagree with this, especially when you are a known member of a project. Adding a +1 with no comment means "I've looked at this patch, and it seems fine to me". Many cores have to budget their time for reviews, and if they see a patch with a few +1s from team members, it is a sign that there is nothing obviously wrong with it, and it might be a good candidate for them to review.
> 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.
This part I totally agree with. It doesn't help to say "something is wrong with this"; you need to spell out what it is that you feel is wrong.
-- Ed Leafe
More information about the openstack-discuss