[dev][all] note to non-core reviewers in all projects
akekane at redhat.com
Tue May 21 16:03:59 UTC 2019
IMO, if you put +1 to a patch you should put some comments like, verified
code as per standards, tested the functionality by applying patch in local
environment, works but refactoring is possible.
The reason behind adding such comments is that it will help to built trust
between core and non-core reviewers. This will also help the non-cores to
speedup their way to become cores.
On Tue, 21 May, 2019, 20:58 Jay Bryant, <jungleboyj 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 do not leave reviews without some sort of comment. When I was
> mentored into doing reviews the expectation was that you at least leave
> some sort of comment with any review. Also, as Graham noted, especially
> for people who are newer to the project this helps give information on
> their review. This is another one of those 'tribal knowledge' items so
> I am not going to get too passionate about +1's with or without comments.
> > 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 obviously is a requirement and it is just rude to -1 with no
> additional direction.
> > That's all I have to say. I now return to my normal sunny disposition.
> > cheers,
> > brian
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the openstack-discuss