ksummit.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Greg KH <greg@kroah.com>
Cc: "ksummit-discuss@lists.linuxfoundation.org"
	<ksummit-discuss@lists.linuxfoundation.org>
Subject: Re: [Ksummit-discuss] [TOPIC] Encouraging more reviewers
Date: Fri, 30 May 2014 09:33:18 +0400	[thread overview]
Message-ID: <1401427998.2163.37.camel@dabdike> (raw)
In-Reply-To: <20140530050220.GA2505@kroah.com>

On Thu, 2014-05-29 at 22:02 -0700, Greg KH wrote:
> On Fri, May 30, 2014 at 08:26:13AM +0400, James Bottomley wrote:
> > On Thu, 2014-05-29 at 16:34 -0700, Greg KH wrote:
> > > > Just preparing it for gerrit takes a long time (around ten minutes per
> > > > patch), which really reduces the volume of trivial patches, just because
> > > > no-one has that much energy (although some would argue it reduces the
> > > > volume too far).
> > > > 
> > > > It's human nature to spend more time on the easy problems, so a flood of
> > > > trivial patches which are "obviously" correct is easier to review and
> > > > include than one patch which alters something deep within mm and needs
> > > > careful thought.  At best, excessively split trivial patches are a
> > > > dilution of our review effort and at worst, they're actively sucking
> > > > people away from tackling the hard problems.
> > > 
> > > I have not heard of these "cleanup" patches taking anyone's review
> > > cycles up for the "harder" patches, do you have examples of it?
> > 
> > Yes, me.
> 
> Then flat out reject them.  Some subsystem maintainers do.  Or, push
> back, like you said later with a "I'll take this if you will maintain
> the driver from now on." :)

I do (I believe I already said that).

> No one is forcing you to take these drivers, but as someone who has
> really old code, I can see how it would get annoying with people hitting
> you with cleanup patches.  Maybe you should just do a "coding style
> cleanup" set of patches yourself one release to get it all in shape then
> not have to worry about it anymore.  That's what I did years ago for the
> USB drivers...

I'm not necessarily promoting my own position, I'm questioning the wider
assertion that all these clean up patches are a good way of getting into
kernel coding.  What worries me the most is that I don't see evidence
that after hundreds of clean up patches anyone moves on to more
substantive issues.  Once we incent people with kudos for patches, why
would you move on to more substantive changes? they're harder and take
more time.  You can turn out many more cleanup type patches in the time
it takes to argue for and produce a well thought out substantive change.
On these metrics we're actually encouraging people not to graduate ...
that's really a bad thing.

We need to design programs for new people that draw them in, not keep
them on the periphery.  That's why I think bug fixing is a better
activity: it means they have to understand some of the code; the change
can be small and isolated, so they don't have to understand all the code
and it often encourages people to find out more about whatever they're
fixing, hence draws them in.

We're way off the topic of encouraging reviewers, but the bottom line
for me is I don't believe starting people out with cleanup type changes
is a good way to get wider engagement.  It is a good way to increase the
number of overall patches, all of which need to be reviewed, hence the
need for more reviewers.

James

  reply	other threads:[~2014-05-30  5:33 UTC|newest]

Thread overview: 166+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-24  9:53 James Bottomley
2014-05-24 11:19 ` Wolfram Sang
2014-05-24 19:18   ` Guenter Roeck
2014-05-25  4:56     ` NeilBrown
2014-05-25  4:57     ` James Bottomley
2014-05-26 15:41       ` Guenter Roeck
2014-05-30 16:05         ` mark gross
2014-05-30 16:45           ` Guenter Roeck
2014-06-01 14:05           ` Wolfram Sang
2014-05-25  8:59     ` Wolfram Sang
2014-05-26 12:23       ` Rafael J. Wysocki
2014-05-26 12:52         ` Wolfram Sang
2014-05-27 17:27   ` Lukáš Czerner
2014-05-27 22:57     ` Rafael J. Wysocki
2014-05-27 22:43       ` Andy Lutomirski
2014-05-27 23:09         ` Rafael J. Wysocki
2014-05-28 14:26       ` Daniel Vetter
2014-05-28 14:32         ` Dan Carpenter
2014-05-28 14:39           ` Daniel Vetter
2014-05-28 16:39             ` Mark Brown
2014-05-28 16:51               ` Mimi Zohar
2014-05-28 17:35                 ` Mark Brown
2014-05-28 17:44                   ` Luck, Tony
2014-05-28 18:38                     ` Mark Brown
2014-05-28 21:32                       ` Thomas Gleixner
2014-05-29  9:28                         ` Li Zefan
2014-05-29 17:41                           ` Greg KH
2014-05-30  2:41                             ` Li Zefan
2014-05-30 17:28                             ` Paul E. McKenney
2014-05-30 23:40                               ` Greg KH
2014-05-31 16:49                                 ` Geert Uytterhoeven
2014-06-01  8:36                                   ` Takashi Iwai
2014-05-31 23:30                               ` Randy Dunlap
2014-05-29 18:43                         ` Steven Rostedt
2014-05-28 22:48               ` Daniel Vetter
2014-05-28 23:17                 ` Laurent Pinchart
2014-05-29 18:45                   ` Steven Rostedt
2014-05-29  7:35             ` Dan Carpenter
2014-05-28 16:05         ` Guenter Roeck
2014-05-28 16:37           ` Mimi Zohar
2014-05-28 16:50             ` Guenter Roeck
2014-05-28 16:20         ` Mimi Zohar
2014-05-28 16:28           ` Josh Triplett
2014-05-28 17:05             ` Jonathan Corbet
2014-05-28 21:59             ` Thomas Gleixner
2014-05-28 23:31               ` josh
2014-05-28 23:55                 ` Thomas Gleixner
2014-05-29  0:39                 ` Mimi Zohar
2014-05-29  0:47                   ` Randy Dunlap
2014-05-29  0:52                     ` Mimi Zohar
2014-05-29  6:13                 ` James Bottomley
2014-05-29 18:58                   ` Steven Rostedt
2014-05-29 23:34                   ` Greg KH
2014-05-30  2:23                     ` Li Zefan
2014-05-30  4:26                     ` James Bottomley
2014-05-30  5:02                       ` Greg KH
2014-05-30  5:33                         ` James Bottomley [this message]
2014-05-30 14:14                           ` John W. Linville
2014-05-30 16:40                           ` Theodore Ts'o
2014-05-30 16:43                             ` Theodore Ts'o
2014-05-30 16:56                           ` [Ksummit-discuss] More productive uses of enthusiastic new kernel developers (was: Re: [TOPIC] Encouraging more reviewers) Theodore Ts'o
2014-05-30 19:54                             ` Shuah Khan
2014-06-02 12:00                               ` Jason Cooper
2014-05-30 20:50                             ` David Woodhouse
2014-05-31  1:44                             ` [Ksummit-discuss] More productive uses of enthusiastic new kernel developers Li Zefan
2014-05-31  1:54                               ` Guenter Roeck
2014-05-31  2:21                                 ` Theodore Ts'o
2014-05-31 22:53                                   ` Rafael J. Wysocki
2014-05-31  2:07                               ` Theodore Ts'o
2014-05-31  3:52                                 ` Greg KH
2014-05-31  4:08                                   ` Guenter Roeck
2014-05-30 23:47                           ` [Ksummit-discuss] [TOPIC] Encouraging more reviewers Greg KH
2014-05-30 11:17                       ` Dan Carpenter
2014-05-31 21:05                         ` Dan Carpenter
2014-05-29 10:31                 ` Daniel Vetter
2014-05-29 18:36                   ` Greg KH
2014-05-29 15:32                 ` Luck, Tony
2014-05-28  5:37     ` Wolfram Sang
2014-05-28 10:06       ` Lukáš Czerner
2014-05-28 13:57         ` Wolfram Sang
2014-05-24 14:24 ` Dan Williams
2014-05-26 12:31   ` Rafael J. Wysocki
2014-05-24 15:50 ` Trond Myklebust
2014-05-24 17:31   ` James Bottomley
2014-05-25  4:15     ` Bjorn Helgaas
2014-05-26 12:38       ` Rafael J. Wysocki
2014-05-27 18:21     ` H. Peter Anvin
2014-05-25  4:17 ` Stephen Rothwell
2014-05-25  8:53   ` Geert Uytterhoeven
2014-05-25  9:11     ` Stephen Rothwell
2014-05-27  8:16     ` Li Zefan
2014-05-25  9:09   ` Wolfram Sang
2014-05-25 22:29 ` Dan Carpenter
2014-05-26 15:53   ` James Bottomley
2014-05-27 14:39     ` Jiri Kosina
2014-05-27 20:53       ` James Bottomley
2014-05-27 21:22         ` Jiri Kosina
2014-05-28  0:10           ` Martin K. Petersen
2014-05-28  0:30             ` Greg KH
2014-05-28 23:25             ` Dan Williams
2014-05-28 23:32               ` Jiri Kosina
2014-05-28 23:47                 ` Dan Williams
2014-05-29  4:01               ` Martin K. Petersen
2014-05-29  5:17                 ` Dan Williams
2014-05-29 23:56                   ` Martin K. Petersen
2014-05-29 23:59                     ` Dan Williams
2014-05-28 23:33             ` Rafael J. Wysocki
2014-05-29  0:35             ` Ben Hutchings
2014-05-29  4:36               ` Martin K. Petersen
2014-05-29 16:46               ` Mark Brown
2014-05-29 21:57                 ` Frank Rowand
2014-05-29 23:12                   ` Mark Brown
2014-05-28  1:10           ` NeilBrown
2014-05-28  5:11           ` James Bottomley
2014-05-26 12:17 ` Rafael J. Wysocki
2014-05-28 18:47 ` Paul Walmsley
2014-05-28 20:15   ` josh
2014-05-29  2:15   ` Rob Herring
2014-05-29  3:34     ` Olof Johansson
2014-05-30  0:52       ` Paul Walmsley
2014-05-29  8:39     ` Jonathan Cameron
2014-05-30  0:47     ` Paul Walmsley
2014-05-30  0:51     ` Paul Walmsley
2014-05-28 18:48 ` [Ksummit-discuss] Reforming Acked-by (was Re: [TOPIC] Encouraging more reviewers) Paul Walmsley
2014-05-28 19:11   ` Mimi Zohar
2014-05-28 19:15     ` John W. Linville
2014-05-28 19:51       ` Mimi Zohar
2014-05-30 14:59       ` Steven Rostedt
2014-05-30 15:10         ` John W. Linville
2014-05-30 21:10           ` James Bottomley
2014-05-30 21:30             ` Steven Rostedt
2014-06-02  2:43             ` Randy Dunlap
2014-06-02  2:53               ` NeilBrown
2014-06-02  3:01                 ` Randy Dunlap
2014-05-28 19:49   ` Guenter Roeck
2014-05-28 20:12   ` josh
2014-05-28 20:22     ` Dmitry Torokhov
2014-05-28 23:02       ` Laurent Pinchart
2014-05-28 23:18         ` Dmitry Torokhov
2014-05-28 23:29           ` Laurent Pinchart
2014-05-29 14:44             ` Christoph Lameter
2014-05-29 14:59               ` Laurent Pinchart
2014-05-29 16:33                 ` Christoph Lameter
2014-05-30 10:58                   ` Laurent Pinchart
2014-05-29 15:58   ` H. Peter Anvin
2014-05-29 18:27   ` Theodore Ts'o
2014-05-29 21:03     ` Rafael J. Wysocki
2014-05-29 21:03       ` Olof Johansson
2014-05-29 23:30         ` Greg KH
2014-05-30  1:12           ` Paul Walmsley
2014-05-30  5:04             ` Greg KH
2014-05-30  5:39               ` James Bottomley
2014-05-30 11:30                 ` Daniel Vetter
2014-05-30 23:39                 ` Greg KH
2014-05-30 10:08           ` Lukáš Czerner
2014-05-30 13:07             ` Jan Kara
2014-05-30 13:41               ` Lukáš Czerner
2014-05-30 15:22               ` Steven Rostedt
2014-05-31  1:30                 ` Li Zefan
2014-05-30 14:34           ` John W. Linville
2014-05-30  0:55         ` Paul Walmsley
2014-05-30 15:17         ` Steven Rostedt
2014-05-30 15:06       ` Steven Rostedt
2014-05-30 21:26         ` Rafael J. Wysocki
2014-05-30 21:29           ` Steven Rostedt
2014-05-30 22:16             ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1401427998.2163.37.camel@dabdike \
    --to=james.bottomley@hansenpartnership.com \
    --cc=greg@kroah.com \
    --cc=ksummit-discuss@lists.linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox