Open Stack

On 05/30/2018 01:42 AM, Zane Bitter wrote:
> On 29/05/18 16:49, Slawomir Kaplonski wrote:
>> Hi,
>>
>>> Wiadomość napisana przez Jay S Bryant <jungleboyj at 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).

hm, "Download" -> copy/paste, and Voilà. Gerrit interface is pretty nice
with the user (I an "old new contributor", never really struggled with
Gerrit itself.. On the other hand, heat, ansible, that's another story :) ).

> 
> 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.

+1 (not core, can't +2, sorry :D)

"-1" is "aggressive".

Cheers,

C.

> 
> 
> 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-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

-- 
Cédric Jeanneret
Software Engineer
DFG:DF

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20180530/f40051b4/attachment.sig>