settingsLogin | Registersettings

[openstack-dev] Garbage patches for simple typo fixes

0 votes

I just wanted to highlight to people that there seems to be a series of
garbage patches in various projects [1] which are basically doing things
like fixing a single typo in a code comment, or very narrowly changing
http to https in links within docs.

Also +1ing ones own changes.

I've been trying to snuff these out in nova, but I see it's basically a
pattern widespread across several projects.

This is the boilerplate comment I give with my -1, feel free to employ
it yourself.

"Sorry but this isn't really a useful change. Fixing typos in code
comments when the context is still clear doesn't really help us, and
mostly seems like looking for padding stats on stackalytics. It's also a
drain on our CI environment.

If you fixed all of the typos in a single module, or in user-facing
documentation, or error messages, or something in the logs, or something
that actually doesn't make sense in code comments, then maybe, but this
isn't one of those things."

I'm not trying to be a jerk here, but this is annoying to the point I
felt the need to say something publicly.

[1] https://review.openstack.org/#/q/author:%255E.*inspur.*

--

Thanks,

Matt


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
asked Sep 26, 2017 in openstack-dev by mriedemos_at_gmail.c (15,720 points)   2 4 10

58 Responses

0 votes

On 22/09/17 14:47 -0400, Doug Hellmann wrote:
Excerpts from Davanum Srinivas (dims)'s message of 2017-09-22 13:47:06 -0400:

Doug,
Howard (cc'ed) already did a bunch of reaching out especially on
wechat. We should request his help.

Howard,
Can you please help with communications and follow up?

Thanks,
Dims

Thanks, Dims and Howard,

I think the problem has reached a point where it would be a good
idea to formalize our approach to outreach. We should track the
patches or patch series identified as problematic, so reviewers
know not to bother with them. We can also track who is contacting
whom (and how) so we don't have a bunch of people replicating work
or causing confusion for people who are trying to contribute. Having
that information will also help us figure out when we need to
escalate by finding the right managers to be talking to.

Let's put together a small team to manage this instead of letting
it continue to cause frustration for everyone.

Count me in! I'm interested in helping with this effort.

Flavio

--
@flaper87
Flavio Percoco


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

responded Sep 25, 2017 by Flavio_Percoco (36,960 points)   3 7 11
0 votes

On 2017-09-22 06:11 PM, Mike Perez wrote:
On 21:21 Sep 21, Matt Riedemann wrote:

I just wanted to highlight to people that there seems to be a series of
garbage patches in various projects [1] which are basically doing things
like fixing a single typo in a code comment, or very narrowly changing http
to https in links within docs.

Also +1ing ones own changes.

I've been trying to snuff these out in nova, but I see it's basically a
pattern widespread across several projects.

This is the boilerplate comment I give with my -1, feel free to employ it
yourself.

"Sorry but this isn't really a useful change. Fixing typos in code comments
when the context is still clear doesn't really help us, and mostly seems
like looking for padding stats on stackalytics. It's also a drain on our CI
environment.

If you fixed all of the typos in a single module, or in user-facing
documentation, or error messages, or something in the logs, or something
that actually doesn't make sense in code comments, then maybe, but this
isn't one of those things."

I'm not trying to be a jerk here, but this is annoying to the point I felt
the need to say something publicly.

[1] https://review.openstack.org/#/q/author:%255E.*inspur.*
I agree with the frustration here. It was mentioned earlier in the thread the
top 5 wanted [1] is a good step in the right direction. I think also the
efforts on the contributor portal [2] are going to be a helpful link to send
people when they make mistakes.

I'm sure some of the people who haven't had this communicated to them yet
aren't aware, so we should all be aware as demonstrated in Matt's boilerplate
comment to be nice when communicating.

[1] - http://governance.openstack.org/tc/reference/top-5-help-wanted.html

When I click this link I see two items. Perhaps the list can be named
'help wanted list' and the point about it being the top 5 or having a
maximum of 5 items can be made in the text. Having a top 5 list with 2
items may confuse the audience folks are trying to redirect here.

Thanks,
Anita.

[2] - http://lists.openstack.org/pipermail/openstack-dev/2017-September/122534.html


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@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@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Sep 25, 2017 by Anita_Kuno (21,320 points)   3 3 4
0 votes

On 2017-09-25 12:12:23 -0400 (-0400), Anita Kuno wrote:
When I click this link I see two items. Perhaps the list can be
named 'help wanted list' and the point about it being the top 5 or
having a maximum of 5 items can be made in the text. Having a top
5 list with 2 items may confuse the audience folks are trying to
redirect here.

The original intent was not to publicize it until we had approved
three more items, but five seems like a somewhat arbitrary number to
me and baking it into the idea has resulted in us postponing
advertisement of good existing content.
--
Jeremy Stanley


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Sep 25, 2017 by Jeremy_Stanley (56,700 points)   3 5 7
0 votes

Sean, I quantified it in 2016 for some of the patches that came in to
Trove; approximately 130 hours of CI time per patch given the number of
hacks that the person took before even getting pep8 to run.

Close to a release boundary, that had very bad results on an already
fragile Trove gate.

-amrith

On Mon, Sep 25, 2017 at 9:42 AM, Sean Dague sean@dague.net wrote:

On 09/25/2017 09:28 AM, Doug Hellmann wrote:

I'm less concerned with the motivation of someone submitting the
patches than I am with their effect. Just like the situation we had
with the bug squash days a year or so ago, if we had a poorly timed
set of these trivial patches coming in at our feature freeze deadline,
it would be extremely disruptive. So to me the fact that we're
seeing them in large batches means we have people who are not fully
engaged with the community and don't understand the impact they're
having. My goal is to reach out and try to improve that engagement,
and try to help them become more fully constructive contributors.

I think that quantifying how big that impact is would be good before
deciding it needs to be a priority to act upon. There are lots of things
that currently swamp our system, and on my back of the envelope math and
spot checking on resources used, these really aren't a big concern.

But it's harder to see that until we really start accounting for CI time
by project / person, and what kinds of things really do consume the system.

    -Sean

--
Sean Dague
http://dague.net


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@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@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Sep 26, 2017 by amrith.kumar_at_gmai (3,580 points)   2 3
0 votes

Sean,

Each fix is a valid change, in and of itself. But what kind of lazy person
would fix (in three patches) the exact same thing, in three places in a
single project?

Or which person would submit the exact same kind of patch multiple times to
change URL's from http:// to https:// in places which are (literally)
comments in the code?

Or submit multiple changes to fix something that is a python stylistic
thing, not enforced by pep8 or some project wide checks for style?
Typically, when I see changes like this, I ask the submitter to make a
corresponding change to the accepted tests (enable an otherwise disabled
change, and show that the tests pass for the whole project). Well, that's
real work and the change gets abandoned.

Those are the kinds of (for lack of a PC word) bad behavior that I think we
should, as a community, reject.

-amrith

On Mon, Sep 25, 2017 at 8:24 AM, Sean Dague sean@dague.net wrote:

On 09/25/2017 07:56 AM, Chris Dent wrote:

On Fri, 22 Sep 2017, Paul Belanger wrote:

This is not a good example of encouraging anybody to contribute to the
project.

Yes. This entire thread was a bit disturbing to read. Yes, I totally
agree that mass patches that do very little are a big cost to
reviewer and CI time but a lot of the responses sound like: "go away
you people who don't understand our special culture and our
important work".

That's not a good look.

Matt's original comment is good in and of itself: I saw a thing,
let's remember to curtail this stuff and do it in a nice way.

But then we generate a long thread about it. It's odd to me that
these threads sometimes draw more people out then discussions about
actually improving the projects.

It's also odd that if OpenStack were small and differently
structured, any self-respecting maintainer would be happy to see
a few typo fixes and generic cleanups. Anything to push the quality
forward is nice. But because of the way we do review and because of
the way we do CI these things are seen as expensive distractions[1].
We're old and entrenched enough now that our tooling enforces our
culture and our culture enforces our tooling.

[1] Note that I'm not denying they are expensive distractions nor
that they need to be managed as such. They are, but a lot of that
is on us.

I was trying to ignore the thread in the hopes it would die out quick.
But torches and pitchforks all came out from the far corners, so I'm
going to push back on that a bit.

I'm not super clear why there is always so much outrage about these
patches. They are fixing real things. When I encounter them, I just
approve them to get them merged quickly and not backing up the review
queue, using more CI later if they need rebasing. They are fixing real
things. Maybe there is a CI cost, but the faster they are merged the
less likely someone else is to propose it in the future, which keeps
down the CI cost. And if we have a culture of just fixing typos later,
then we spend less CI time on patches the first time around with 2 or 3
iterations catching typos.

I think the concern is the ascribed motive for why people are putting
these up. That's fine to feel that people are stat padding (and that too
many things are driven off metrics). But, honestly, that's only
important if we make it important. Contributor stats are always going to
be pretty much junk stats. They are counting things to be the same which
are wildly variable in meaning (number of patches, number of Lines of
Code).

My personal view is just merge things that fix things that are wrong,
don't care why people are doing it. If it gets someone a discounted
ticket somewhere, so be it. It's really not any skin off our back in the
process.

If people are deeply concerned about CI resources, step one is to get
some better accounting into the existing system to see where resources
are currently spent, and how we could ensure that time is fairly spread
around to ensure maximum productivity by all developers.

    -Sean

--
Sean Dague
http://dague.net


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@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@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Sep 26, 2017 by amrith.kumar_at_gmai (3,580 points)   2 3
0 votes

Sean Dague wrote:
I think the concern is the ascribed motive for why people are putting
these up. That's fine to feel that people are stat padding (and that too
many things are driven off metrics). But, honestly, that's only
important if we make it important. Contributor stats are always going to
be pretty much junk stats. They are counting things to be the same which
are wildly variable in meaning (number of patches, number of Lines of
Code).

If this is a real thing (which I don't know if it is, but I could
believe that it is) due to management or other connecting those stats to
involvement (and likely at some point $$) why don't we just turn off
http://stackalytics.com/ or make it require a launchpad login (make it a
little harder to access) or put a big warning banner on it that says
these stats are not-representative of much of anything...

The hard part is it's not us as a community deciding 'important if we
make it important' because such motives are not directly associated to
contributors but instead may or may not be connected to management of
said contributor (and management of the contributors that are paid to
work on openstack has always been in the background somewhere, like a
ghost...).

-Josh


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Sep 26, 2017 by harlowja_at_fastmail (16,200 points)   2 5 8
0 votes

On 9/23/2017 10:11 AM, Doug Hellmann wrote:
Excerpts from Huang Zhiteng's message of 2017-09-23 10:00:00 +0800:

On Sat, Sep 23, 2017 at 8:34 AM, Zhipeng Huang zhipengh512@gmail.com wrote:

Hi Paul,

Unfortunately I know better on this matter and it is not the matter of topic
dispute as many people on this thread who has been disturbed and annoyed by
the padding/trolling.

So yes I'm sticking with stupid because it hurts the OpenStack community as
a whole and hurts the reputation of the dev community from my country which
in large are great people with good hearts and skills.

I'm not giving even an inch of the benefit of doubt to these padding
activities and people behind it.
Hi Zhipeng,

Not sure how much you have been involved in the dev community in
China, but it's now a good time to talk to those companies (in public
or private) and ask them to stop encourage their developers to submit
such changes.
I would prefer to set up a system where we can have those sorts of
conversations in private, to encourage people to contribute
constructively instead of shaming them.

Doug
+2

This, in some cases, may be due to people trying to pad their numbers. 
Perhaps it is just people who do not yet know the best way to help out
and want to do something.

I agree with Doug's comments about this needing to be done in private
and with Ildiko's comments on providing mentoring.  This is something I
will consider as I put together the on-boarding education for the Sydney
Summit.

From a Cinder standpoint I have been trying to be inclusive and not
block things unless it just appears to be blatantly pointless. Trying to
keep on the side of community inclusion.

Jay

On Sat, Sep 23, 2017 at 8:16 AM, Paul Belanger pabelanger@redhat.com
wrote:

On Fri, Sep 22, 2017 at 10:26:09AM +0800, Zhipeng Huang wrote:

Let's not forget the epic fail earlier on the "contribution.rst fix"
that
almost melt down the community CI system.

For any companies that are doing what Matt mentioned, please be aware
that
the dev community of the country you belong to is getting hurt by your
stupid activity.

Stop patch trolling and doing something meaningful.

Sorry, but I found this comment over the line. Just because you disagree
with
the $topic at hand, doesn't mean you should default to calling it
'stupid'. Give
somebody the benefit of not knowing any better.

This is not a good example of encouraging anybody to contribute to the
project.

-Paul

On Fri, Sep 22, 2017 at 10:21 AM, Matt Riedemann mriedemos@gmail.com
wrote:

I just wanted to highlight to people that there seems to be a series
of
garbage patches in various projects [1] which are basically doing
things
like fixing a single typo in a code comment, or very narrowly changing
http
to https in links within docs.

Also +1ing ones own changes.

I've been trying to snuff these out in nova, but I see it's basically
a
pattern widespread across several projects.

This is the boilerplate comment I give with my -1, feel free to employ
it
yourself.

"Sorry but this isn't really a useful change. Fixing typos in code
comments when the context is still clear doesn't really help us, and
mostly
seems like looking for padding stats on stackalytics. It's also a
drain on
our CI environment.

If you fixed all of the typos in a single module, or in user-facing
documentation, or error messages, or something in the logs, or
something
that actually doesn't make sense in code comments, then maybe, but
this
isn't one of those things."

I'm not trying to be a jerk here, but this is annoying to the point I
felt
the need to say something publicly.

[1] https://review.openstack.org/#/q/author:%255E.*inspur.*

--

Thanks,

Matt


OpenStack Development Mailing List (not for usage questions)
Unsubscribe:
OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

--
Zhipeng (Howard) Huang

Standard Engineer
IT Standard & Patent/IT Product Line
Huawei Technologies Co,. Ltd
Email: huangzhipeng@huawei.com
Office: Huawei Industrial Base, Longgang, Shenzhen

(Previous)
Research Assistant
Mobile Ad-Hoc Network Lab, Calit2
University of California, Irvine
Email: zhipengh@uci.edu
Office: Calit2 Building Room 2402

OpenStack, OPNFV, OpenDaylight, OpenCompute Aficionado


OpenStack Development Mailing List (not for usage questions)
Unsubscribe:
OpenStack-dev-request@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@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

--
Zhipeng (Howard) Huang

Standard Engineer
IT Standard & Patent/IT Product Line
Huawei Technologies Co,. Ltd
Email: huangzhipeng@huawei.com
Office: Huawei Industrial Base, Longgang, Shenzhen

(Previous)
Research Assistant
Mobile Ad-Hoc Network Lab, Calit2
University of California, Irvine
Email: zhipengh@uci.edu
Office: Calit2 Building Room 2402

OpenStack, OPNFV, OpenDaylight, OpenCompute Aficionado


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@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@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@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Sep 26, 2017 by jungleboyj_at_gmail. (2,360 points)   2
0 votes

On 9/25/2017 7:24 AM, Sean Dague wrote:
On 09/25/2017 07:56 AM, Chris Dent wrote:

On Fri, 22 Sep 2017, Paul Belanger wrote:

This is not a good example of encouraging anybody to contribute to the
project.
Yes. This entire thread was a bit disturbing to read. Yes, I totally
agree that mass patches that do very little are a big cost to
reviewer and CI time but a lot of the responses sound like: "go away
you people who don't understand our special culture and our
important work".

That's not a good look.

Matt's original comment is good in and of itself: I saw a thing,
let's remember to curtail this stuff and do it in a nice way.

But then we generate a long thread about it. It's odd to me that
these threads sometimes draw more people out then discussions about
actually improving the projects.

It's also odd that if OpenStack were small and differently
structured, any self-respecting maintainer would be happy to see
a few typo fixes and generic cleanups. Anything to push the quality
forward is nice. But because of the way we do review and because of
the way we do CI these things are seen as expensive distractions[1].
We're old and entrenched enough now that our tooling enforces our
culture and our culture enforces our tooling.

[1] Note that I'm not denying they are expensive distractions nor
that they need to be managed as such. They are, but a lot of that
is on us.
I was trying to ignore the thread in the hopes it would die out quick.
But torches and pitchforks all came out from the far corners, so I'm
going to push back on that a bit.

I'm not super clear why there is always so much outrage about these
patches. They are fixing real things. When I encounter them, I just
approve them to get them merged quickly and not backing up the review
queue, using more CI later if they need rebasing. They are fixing real
things. Maybe there is a CI cost, but the faster they are merged the
less likely someone else is to propose it in the future, which keeps
down the CI cost. And if we have a culture of just fixing typos later,
then we spend less CI time on patches the first time around with 2 or 3
iterations catching typos.
Thank you for saying what I failed to say in my most recent response.  I
know some people don't care about typos, etc but they are things that
make us look like a lower quality community.  It is stuff to fix and I
think we are wasting more resource in this discussion than just getting
the patches through.
I think the concern is the ascribed motive for why people are putting
these up. That's fine to feel that people are stat padding (and that too
many things are driven off metrics). But, honestly, that's only
important if we make it important. Contributor stats are always going to
be pretty much junk stats. They are counting things to be the same which
are wildly variable in meaning (number of patches, number of Lines of
Code).

My personal view is just merge things that fix things that are wrong,
don't care why people are doing it. If it gets someone a discounted
ticket somewhere, so be it. It's really not any skin off our back in the
process.
+2  I am going to assume the voice of reason has been heard and not
frustrate myself further with this thread.
If people are deeply concerned about CI resources, step one is to get
some better accounting into the existing system to see where resources
are currently spent, and how we could ensure that time is fairly spread
around to ensure maximum productivity by all developers.

-Sean


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Sep 26, 2017 by jungleboyj_at_gmail. (2,360 points)   2
...