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

Julia Kreger juliaashleykreger at gmail.com
Tue May 21 15:13:59 UTC 2019


I've likely just not had enough coffee this morning to grok this
thread, but I'd like to point out that we have a guide for how to
review[0]. I've found this document very useful to help provide people
with context and re-calibrate their review behaviors such that they
are not detrimental to the community and collaboration.

/me goes back to coffeee

[0]: https://docs.openstack.org/project-team-guide/review-the-openstack-way.html.

On Tue, May 21, 2019 at 8:09 AM Slawomir Kaplonski <skaplons at redhat.com> wrote:
>
> Hi,
>
> > On 21 May 2019, at 16:47, Mohammed Naser <mnaser at vexxhost.com> wrote:
> >
> > On Tue, May 21, 2019 at 9:37 PM Brian Rosmaita
> > <rosmaita.fossdev at gmail.com> wrote:
> >>
> >> 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.
> >
> > I feel a bit different about this.
> >
> > Most cores probably leave a +2 with no comment to merge code, the implication
> > that a non-core needs to add more comments to it seems a bit 'unfair'.
> >
> > I think this is something where we need to put our judgement in.  It may seem
> > that we get a bunch of +1s, but if there is also other -1s that have value and
> > comments, then I think it's okay that we get +1s.
> >
> > tl;dr: I don't think we should say "if you're going to give a +1,
> > leave a comment
> > why you agree", IMHO.
>
> I agree with that. +1 without comment still can have some value.
> But we should say “if you’re going to gibe a -1, please leave a comment”. That’s really important IMO.
>
> >
> >> 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
> >>
> >
> >
> > --
> > Mohammed Naser — vexxhost
> > -----------------------------------------------------
> > D. 514-316-8872
> > D. 800-910-1726 ext. 200
> > E. mnaser at vexxhost.com
> > W. http://vexxhost.com
>
>> Slawek Kaplonski
> Senior software engineer
> Red Hat
>
>



More information about the openstack-discuss mailing list