[openstack-dev] [tc][all] A culture change (nitpicking)
Ian Wells
ijw.ubuntu at cack.org.uk
Tue May 29 19:17:12 UTC 2018
If your nitpick is a spelling mistake or the need for a comment where
you've pretty much typed the text of the comment in the review comment
itself, then I have personally found it easiest to use the Gerrit online
editor to actually update the patch yourself. There's nothing magical
about the original submitter, and no point in wasting your time and theirs
to get them to make the change. That said, please be a grown up; if you're
changing code or messing up formatting enough for PEP8 to be a concern,
it's your responsibility, not the original submitter's, to fix it. Also,
do all your fixes in one commit if you don't want to make Zuul cry.
--
Ian.
On 29 May 2018 at 09:00, Neil Jerram <neil at tigera.io> wrote:
> From my point of view as someone who is still just an occasional
> contributor (in all OpenStack projects other than my own team's networking
> driver), and so I think still sensitive to the concerns being raised here:
>
> - Nits are not actually a problem, at all, if they are uncontroversial and
> quick to deal with. For example, if it's a point of English, and most
> English speakers would agree that a correction is better, it's quick and no
> problem for me to make that correction.
>
> - What is much more of a problem is:
>
> - Anything that is more a matter of opinion. If a markup is just the
> reviewer's personal opinion, and they can't say anything to explain more
> objectively why their suggestion is better, it would be wiser to defer to
> the contributor's initial choice.
>
> - Questioning something unconstructively or out of proportion to the
> change being made. This is a tricky one to pin down, but sometimes I've
> had comments that raise some random left-field question that isn't really
> related to the change being made, or where the reviewer could have done a
> couple minutes research themselves and then either made a more precise
> comment, or not made their comment at all.
>
> - Asking - implicitly or explicitly - the contributor to add more
> cleanups to their change. If someone usefully fixes a problem, and their
> fix does not of itself impair the quality or maintainability of the
> surrounding code, they should not be asked to extend their fix so as to fix
> further problems that a more regular developer may be aware of in that
> area, or to advance a refactoring / cleanup that another developer has in
> mind. (At least, not as part of that initial change.)
>
> (Obviously the common thread of those problem points is taking up more
> time; psychologically I think one of the things that can turn a contributor
> away is the feeling that they've contributed a clearly useful thing, yet
> the community is stalling over accepting it for reasons that do not appear
> clearcut.)
>
> Hoping this is vaguely helpful...
> Neil
>
>
> On Tue, May 29, 2018 at 4:35 PM Amy Marrich <amy at demarco.com> wrote:
>
>> If I have a nit that doesn't affect things, I'll make a note of it and
>> say if you do another patch I'd really like it fixed but also give the
>> patch a vote. What I'll also do sometimes if I know the user or they are
>> online I'll offer to fix things for them, that way they can see what I've
>> done, I've sped things along and I haven't caused a simple change to take a
>> long amount of time and reviews.
>>
>> I think this is a great addition!
>>
>> Thanks,
>>
>> Amy (spotz)
>>
>> On Tue, May 29, 2018 at 6:55 AM, Julia Kreger <
>> juliaashleykreger at gmail.com> wrote:
>>
>>> During the Forum, the topic of review culture came up in session after
>>> session. During these discussions, the subject of our use of nitpicks
>>> were often raised as a point of contention and frustration, especially
>>> by community members that have left the community and that were
>>> attempting to re-engage the community. Contributors raised the point
>>> of review feedback requiring for extremely precise English, or
>>> compliance to a particular core reviewer's style preferences, which
>>> may not be the same as another core reviewer.
>>>
>>> These things are not just frustrating, but also very inhibiting for
>>> part time contributors such as students who may also be time limited.
>>> Or an operator who noticed something that was clearly a bug and that
>>> put forth a very minor fix and doesn't have the time to revise it over
>>> and over.
>>>
>>> While nitpicks do help guide and teach, the consensus seemed to be
>>> that we do need to shift the culture a little bit. As such, I've
>>> proposed a change to our principles[1] in governance that attempts to
>>> capture the essence and spirit of the nitpicking topic as a first
>>> step.
>>>
>>> -Julia
>>> ---------
>>> [1]: https://review.openstack.org/570940
>>>
>>> ____________________________________________________________
>>> ______________
>>> 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
>>>
>>
>> ____________________________________________________________
>> ______________
>> 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
>>
>
> __________________________________________________________________________
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20180529/5ec2bcd3/attachment.html>
More information about the OpenStack-dev
mailing list