On 29/05/18 16:49, Slawomir Kaplonski wrote:
Hi,
Wiadomość napisana przez Jay S Bryant <jungleb...@gmail.com> w dniu 29.05.2018,
o godz. 22:25:
Maybe it would be different now that I am a Core/PTL but in the past I had been
warned to be careful as it could be misinterpreted if I was changing other
people's patches or that it could look like I was trying to pad my numbers. (I
am a nit-picker though I do my best not to be.
Exactly. I remember when I was doing my first patch (or one of first patches)
and someone pushed new PS with some very small nits fixed. I was a bit confused
because of that and I was thinking why he did it instead of me?
Now it’s of course much more clear for me but for someone who is new
contributor I think that this might be confusing. Maybe such person should at
least remember to explain in comment why he pushed new PS and that’s not
„stealing” work of original author :)
Another issue is that if the original author needs to rev the patch
again for any reason, they then need to figure out how to check out the
modified patch. This requires a fairly sophisticated knowledge of both
git and gerrit, which isn't a problem for those of us who have been
using them for years but is potentially a nightmarish introduction for a
relatively new contributor. Sometimes it's the right choice though
(especially if the patch owner hasn't been seen for a while).
A follow-up patch is a good alternative, unless of course it conflicts
with another patch in the series.
+1 with a comment can also get you a long way - it indicates that you've
reviewed the whole patch and have found only nits to quibble with. If
you're a core reviewer, another core could potentially +2/+A on a
subsequent patch set with the nits addressed if they felt it
appropriate, and even if they don't you'll have an easy re-review when
you follow up.
We have lots of tools in our toolbox that are less blunt than -1. Let's
save -1 for when major work is required and/or the patch as written
would actually break something.
Since I am replying to this thread, Julia also mentioned the situation
where two core reviewers are asking for opposite changes to a patch. It
is never ever ever the contributor's responsibility to resolve a dispute
between two core reviewers! If you see a core reviewer's advice on a
patch and you want to give the opposite advice, by all means take it up
immediately - with *the other core reviewer*. NOT the submitter.
Preferably on IRC and not in the review. You work together every day,
you can figure it out! A random contributor has no chance of parachuting
into the middle of that dynamic and walking out unscathed, and they
should never be asked to.
cheers,
Zane.
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
