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

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.

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
responded Sep 22, 2017 by Zhipeng_Huang (6,720 points)   2 3 3
0 votes

+1000

Thanks for bringing this up, I fully agree that we need to do something about it.

Some time ago I even had an idea of creating a case when we intentionally exclude a person from a team for constantly doing things like this and ignoring our comments. Although I understand it slightly conflicts with our openness principle.. It’s just really tempting quite often.

Renat Akhmerov
@Nokia

On 22 Sep 2017, 09:26 +0700, Zhipeng Huang zhipengh512@gmail.com, 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.

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
responded Sep 22, 2017 by renat.akhmerov_at_gm (5,640 points)   2 2 3
0 votes

Wow, Matt, that's excellent timing.
Day for day, an exact year after the last thread of this kind 1.

Thanks for speaking up, I didn't want to it'd seem like encouraging a
stereotype or prejudice towards certain kind of contributions (or
contributors) but I've also been rolling my eyes a lot recently.
It does look like those users are just seeking to pad statistics, but
this isn't the first time this kind of topic comes up.

Some other contributors with similar patterns:

There are some thoughts in the thread from last year but I don't think
any concrete measures were put in place to encourage better and more
meaningful contributions.

David Moreau Simard
Senior Software Engineer | OpenStack RDO

dmsimard = [irc, github, twitter]

On Thu, Sep 21, 2017 at 10:21 PM, 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


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 22, 2017 by dms_at_redhat.com (3,780 points)   3 4
0 votes

Thanks for being brave enough to say it publicly. Not sure how many
more times I can stomach reading such classic patches as "Optimize the
link address" or "Replace six.iteritems() with .items()".

Yes, it is still possible for us to be an open community while also
minimizing the amount of useless patches.

Messages like the sample Matt provided are part of the solution.
Resisting the urge to -2/force-abandon without explanation is part of
the solution, too. But they aren't the whole solution.

The TC's emerging Top 5 help-wanted list is a step in the right
direction towards solving this problem. Let's publicize the crap out
of that. And we can go further, with project-specific help wanted
lists. And how about a better way to identify and promote issues which
are both low-hanging fruit AND important (they aren't actually that
rare!).

Turning towards more radical solutions: 1) Bake something into
git-review which will print out some agreed-up community guidelines
for what constitutes a useful patch, as well as any
repository-specific guidelines. To keep it reasonable, only show the
message the first time a contributor submits to that repository. 2)
Register fingerprints of common unhelpful patches and have a bot
similar to Elastic Recheck automatically review and comment. 3) Delay
spin-up of resource-intensive/long-running CI jobs until after some
initial review has been added or time has passed. Authorized
contributors, not necessarily synonymous with cores, can override the
delay if there's a critical patch which needs to get through the queue
quickly.

Those are just some ideas off the top of my head, so feel free to tear me apart.

Looking forward to seeing where this conversation goes this time around.

On Thu, Sep 21, 2017 at 10:21 PM, 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


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 22, 2017 by jeremyfreudberg_at_g (500 points)  
0 votes

On Fri, 22 Sep 2017, at 12: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.

For what it's worth, I agree. A few days ago I gave a -1 and commented
on around 50 patches which were adding --- to the top of generally two
yaml files: one was a template the other was a test.

Another patchset removed a single space from the end of a line of a
comment.

After my comments they ware all abandoned.

Given the waste of resources, I can't help but wonder if we should be
re-visiting the way initial check gate is kicked off?

Should someone else have to do an initial +1? (Acknowledging that this
could be a colleague and other offender.)

Or could the gate be smarter about the types of changes (like checking
for one liners or changes to comments, etc)?

Or at least there should be a way for anyone to kill a review if it is
clearly a waste of resources?

Or detect patch bombing across projects.

Sadly people are going to abuse this, although probably generally out of
ignorance. However, how can we be a) smarter about the gate, and b) have
a big stick handy.

Perhaps when developers sign up to the CLA, this issue should be in big
bold writing and we can use that as a stick to disable people's access
to Gerrit? That should be a pretty big incentive to behave, it's hard to
do your job when your access has been removed (and only reinstated after
a specified process).

Anyway just some thoughts.

Cheers,
-c


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 22, 2017 by Chris_Smart (300 points)  
0 votes

On 09/21/2017 10:21 PM, 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.*

The boilerplate is helpful but have we considered putting something
along these lines in official documentation so that reviewers can just
point to it? It should then be clear to all that negative reviews on
these grounds are not simply a function of the individual reviewer's
judgment or personality.

FWIW I think it is better not to attribute motivation in these cases.
Perhaps the code submitter is trying to pad stats, but perhaps they are
just a new contributor trying to learn the process with a "harmless"
patch, or just a compulsive clean-upper who hasn't thought through the
costs in reviewer time and CI resources.


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 22, 2017 by Tom_Barron (980 points)   1
0 votes

Thanks Matt for highlighting this (again). Please also see

http://openstack.markmail.org/thread/iaqsha7hbiitwqe2
http://markmail.org/thread/k62gcehxg6gv5ep4
http://markmail.org/thread/n753w3wljii67yug

When can we take some concrete action to stop these same kinds of things
from coming up again and again?

-amrith

On Fri, Sep 22, 2017 at 8:10 AM, Tom Barron tpb@dyncloud.net wrote:

On 09/21/2017 10:21 PM, 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.*

The boilerplate is helpful but have we considered putting something
along these lines in official documentation so that reviewers can just
point to it? It should then be clear to all that negative reviews on
these grounds are not simply a function of the individual reviewer's
judgment or personality.

FWIW I think it is better not to attribute motivation in these cases.
Perhaps the code submitter is trying to pad stats, but perhaps they are
just a new contributor trying to learn the process with a "harmless"
patch, or just a compulsive clean-upper who hasn't thought through the
costs in reviewer time and CI resources.


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 22, 2017 by amrith.kumar_at_gmai (3,580 points)   2 3
0 votes

Excerpts from Tom Barron's message of 2017-09-22 08:10:35 -0400:

On 09/21/2017 10:21 PM, 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.*

The boilerplate is helpful but have we considered putting something
along these lines in official documentation so that reviewers can just
point to it? It should then be clear to all that negative reviews on
these grounds are not simply a function of the individual reviewer's
judgment or personality.

That's a good idea. How about adding a "Contribution Guidelines" section
to https://docs.openstack.org/project-team-guide/open-development.html
with this and other tips?

FWIW I think it is better not to attribute motivation in these cases.
Perhaps the code submitter is trying to pad stats, but perhaps they are
just a new contributor trying to learn the process with a "harmless"
patch, or just a compulsive clean-upper who hasn't thought through the
costs in reviewer time and CI resources.

Good points.

Doug


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 22, 2017 by Doug_Hellmann (87,520 points)   3 4 9
0 votes

On 9/22/2017 7:10 AM, Tom Barron wrote:
FWIW I think it is better not to attribute motivation in these cases.
Perhaps the code submitter is trying to pad stats, but perhaps they are
just a new contributor trying to learn the process with a "harmless"
patch, or just a compulsive clean-upper who hasn't thought through the
costs in reviewer time and CI resources.

I agree. However, the one that set me off last night was a person from
one company who I've repeatedly -1ed the same types of patches in nova
for weeks, including on stable branches, and within 10 minutes of each
other across several repos, so it's clearly part of some daily routine.
That's what prompted me to send something to the mailing list.

--

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
responded Sep 22, 2017 by mriedemos_at_gmail.c (15,720 points)   2 4 10
0 votes

On 2017-09-22 08:30:21 -0400 (-0400), Amrith Kumar wrote:
[...]
When can we take some concrete action to stop these same kinds of
things from coming up again and again?

Technical solutions to social problems rarely do more than increase
complexity for everyone involved. Communication, documentation and
education are likely our best options here, but I still question the
degree to which the people pushing these sorts of contributions will
actually get the message since it's obvious they aren't engaging
meaningfully with the community before attempting to contribute.

Could demographic profiling help us figure out the primary
motivation (whether it's testing the waters, stats padding or
employers pushing their staff to contribute patches before they've
had time to actually assimilate our community norms and
expectations)?
--
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 22, 2017 by Jeremy_Stanley (56,700 points)   3 5 7
...