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 <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).
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-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Cédric Jeanneret Software Engineer DFG:DF
signature.asc
Description: OpenPGP digital signature
__________________________________________________________________________ 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
