On 05/29/2018 03:43 PM, Davanum Srinivas wrote:
Agree with Ian here.
Also another problem that comes up is: "Why are you touching *MY*
review?" (probably coming from the view where stats - and stackalytics
leaderboard position is important). So i guess we ask permission
before editing (or) file a follow up later (or) just tell folks that
this is ok to do!!
I think Stackalytics is evil and should be killed with fire. It
encourages all kinds of pathological behavior, this being one prime
example. Having worked as a core reviewer, I find zero value from the
project. We know who is contributing code and who is doing reviews
without some robot to tell us.
-Ben
Hoping engaging with them will solve yet another issue is someone
going around filing the same change in a dozen projects (repeatedly!),
but that may be wishful thinking.
-- Dims
On Tue, May 29, 2018 at 12:17 PM, Ian Wells <ijw.ubu...@cack.org.uk> wrote:
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 <n...@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 <a...@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
<juliaashleykre...@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-requ...@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-requ...@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-requ...@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-requ...@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-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
