From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>,
ksummit-discuss@lists.linuxfoundation.org
Subject: Re: [Ksummit-discuss] [MAINTAINERS SUMMIT] Bug-introducing patches
Date: Wed, 12 Sep 2018 15:10:01 +0200 [thread overview]
Message-ID: <20180912131001.GG2760@piout.net> (raw)
In-Reply-To: <3070742.JVHc92YBtH@avalon>
On 12/09/2018 15:53:59+0300, Laurent Pinchart wrote:
> Hi Thomas,
>
> On Wednesday, 12 September 2018 15:29:25 EEST Thomas Gleixner wrote:
> > On Wed, 12 Sep 2018, Laurent Pinchart wrote:
> > > On Wednesday, 12 September 2018 14:55:44 EEST Geert Uytterhoeven wrote:
> > >> Good. Then this discussion wasn't targeted to the SCSI people, but to
> > >> other maintainers pushing brown paper bags and other trivial breakages
> > >> they should have caught beforehand to linux-next ;-)
> > >
> > > That's a behaviour that has been annoying me lately, maintainers should
> > > have no special privilege when it comes to pushing code upstream. All
> > > patches should be posted publicly, given enough time to be reviewed, and
> > > review comments should be addressed before anything is merged to a -next
> > > branch. Unfortunately that's not always the case :-S
> >
> > Come on. Do you really expect me to wait for review when I fix up the
> > internal testing/ 0-day fallout which is often enough something trivial?
> > Do you really expect me to wait for review when I worked with a bug
> > reporter to decode something and have a 100% explanation that it fixes the
> > root cause and not the symptom?
> >
> > 1) Our review capacity is small enough already, so we don't have to
> > throw more stuff out for review.
> >
> > 2) With that modus, bugs will stay unfixed way longer and merging of code
> > will even be more delayed.
>
> I don't expect to wait for review forever, but I expect maintainers to give an
> opportunity to reviewers to review patches. We obviously need to consider the
> balance between review opportunity and problems (such as build breakages) that
> could affect hundreds of developers if left unfixed even for a few days.
>
> Too often I've noticed changes to code I maintain that introduced bugs or
> other issues, performed by a maintainer who didn't even bother to post the
> patch before pushing to to his -next branch, who didn't CC me (I could take
> part of the blame for not reading mailing lists with enough attention, but the
> volume is very high), or, possibly worse, who sent a patch out, received my
> review on the same day, and completely ignored it. The last issue is very
> demotivating for reviewers. Those changes were not at all urgent, some of them
> were "cleanups", or replacement of a deprecated API by a new one. That's very
> different than fixing a build breakage in -next which clearly can't wait.
>
> > If I don't have special rights as a maintainer and you don't trust me that
> > I use my common sense when I'm using these special rights, then you
> > degraded me to a patch juggling monkey. On the day this happens, I'll step
> > down.
>
> Maintainers are much more than patch juggling monkeys, otherwise they could be
> replaced by machines. I believe that maintainers are given the huge
> responsibility of taking care of their community. Fostering a productive work
> environment, attracting (and keeping) talented developers and reviewers is a
> huge and honourable task, and gets my full respect. On top of that, if a
> maintainer has great technical skills, it's even better, and I've learnt a lot
> from talented maintainers over the time. I however believe that technical
> skills are not an excuse for not leading by example and showing what the good
> practices are by applying them.
>
> (This goes without saying, but even better when said explicitly, there's not
> judgment about your or any particular maintainer's technical or non-technical
> skills here)
>
> > > I would even go as far as saying that all patches should have Reviewed-by
> > > or Acked-by tag, without enforcing that rule too strictly (I'm thinking
> > > in particular about drivers that only a single person cares about, it's
> > > sometimes hard to get patches reviewed).
> >
> > If we enforce that, then a large part of reviewed-by and acked-by tags will
> > just come from coworkers or other affiliates and have no value at all.
>
> That's a concern I share, and one of the reasons why I have my doubts about
> some of the maintainership experiments in the DRM subsystem. The proponents of
> the changes there pointed out to me that development has sped up as a result,
> but I think the costs associated with the acceleration haven't been fully
> evaluated.
>
> > That's anyway a growing disease that patches already carry reviewed tags
> > when they are posted the first time and then you look at them and they have
> > at least one easy to spot or easy to detect by tools bug in them.
>
> Do you get bothered that they carry a tag when they are posted the first time,
> or only that they do so *and* have clear problems ?
>
I guess the issue is that it is very difficult to trust reviewed-by or
acked-by tags that are coming from coworkers/affiliates, especially more
when the review didn't happen publicly. From my point of view, that
review may or may not have happened.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
next prev parent reply other threads:[~2018-09-12 13:10 UTC|newest]
Thread overview: 138+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-04 20:16 [Ksummit-discuss] [MAINTAINERS SUMMIT] Bug-introducing patches Sasha Levin
2018-09-04 20:53 ` Daniel Vetter
2018-09-05 14:17 ` Steven Rostedt
2018-09-07 0:51 ` Sasha Levin
2018-09-07 1:09 ` Steven Rostedt
2018-09-07 20:12 ` Greg KH
2018-09-07 21:12 ` Greg KH
2018-09-07 1:09 ` Linus Torvalds
2018-09-07 1:49 ` Sasha Levin
2018-09-07 2:31 ` Linus Torvalds
2018-09-07 2:45 ` Steven Rostedt
2018-09-07 3:43 ` Linus Torvalds
2018-09-07 8:52 ` Daniel Vetter
2018-09-07 8:40 ` Geert Uytterhoeven
2018-09-07 9:07 ` Daniel Vetter
2018-09-07 9:28 ` Geert Uytterhoeven
2018-09-07 17:05 ` Olof Johansson
2018-09-07 14:54 ` Sasha Levin
2018-09-07 15:52 ` Linus Torvalds
2018-09-07 16:17 ` Linus Torvalds
2018-09-07 21:39 ` Mauro Carvalho Chehab
2018-09-09 12:50 ` Stephen Rothwell
2018-09-10 20:05 ` Tony Lindgren
2018-09-10 19:43 ` Sasha Levin
2018-09-10 20:45 ` Steven Rostedt
2018-09-10 21:20 ` Guenter Roeck
2018-09-10 21:46 ` Steven Rostedt
2018-09-10 23:03 ` Eduardo Valentin
2018-09-10 23:13 ` Steven Rostedt
2018-09-11 15:42 ` Steven Rostedt
2018-09-11 17:40 ` Tony Lindgren
2018-09-11 17:47 ` James Bottomley
2018-09-11 18:12 ` Eduardo Valentin
2018-09-11 18:17 ` Geert Uytterhoeven
2018-09-12 15:15 ` Eduardo Valentin
2018-09-11 18:19 ` James Bottomley
2018-09-12 15:17 ` Eduardo Valentin
2018-09-11 18:39 ` Steven Rostedt
2018-09-11 20:09 ` James Bottomley
2018-09-11 20:31 ` Steven Rostedt
2018-09-11 22:53 ` James Bottomley
2018-09-11 23:04 ` Sasha Levin
2018-09-11 23:11 ` James Bottomley
2018-09-11 23:20 ` Sasha Levin
2018-09-12 15:41 ` Eduardo Valentin
2018-09-11 23:22 ` Tony Lindgren
2018-09-11 23:29 ` James Bottomley
2018-09-12 11:55 ` Geert Uytterhoeven
2018-09-12 12:03 ` Laurent Pinchart
2018-09-12 12:29 ` Thomas Gleixner
2018-09-12 12:53 ` Laurent Pinchart
2018-09-12 13:10 ` Alexandre Belloni [this message]
2018-09-12 13:30 ` Thomas Gleixner
2018-09-12 23:16 ` Laurent Pinchart
2018-09-12 14:11 ` Thomas Gleixner
2018-09-19 8:26 ` Laurent Pinchart
2018-09-20 9:02 ` Rafael J. Wysocki
2018-09-20 10:10 ` Laurent Pinchart
2018-09-20 11:00 ` Daniel Vetter
2018-09-20 11:08 ` Laurent Pinchart
2018-09-20 11:49 ` Daniel Vetter
2018-09-12 12:36 ` James Bottomley
2018-09-12 13:38 ` Guenter Roeck
2018-09-12 13:59 ` Tony Lindgren
2018-09-12 10:04 ` Mark Brown
2018-09-12 20:24 ` Steven Rostedt
2018-09-12 20:29 ` Sasha Levin
2018-09-13 0:19 ` Stephen Rothwell
2018-09-13 11:39 ` Mark Brown
2018-09-19 6:27 ` Stephen Rothwell
2018-09-19 17:24 ` Mark Brown
2018-09-19 21:42 ` Stephen Rothwell
2018-09-11 0:49 ` Stephen Rothwell
2018-09-11 1:01 ` Al Viro
2018-09-11 0:47 ` Stephen Rothwell
2018-09-11 17:35 ` Linus Torvalds
2018-09-11 0:43 ` Stephen Rothwell
2018-09-11 16:49 ` Guenter Roeck
2018-09-11 17:47 ` Guenter Roeck
2018-09-11 11:18 ` Mark Brown
2018-09-11 17:02 ` Guenter Roeck
2018-09-11 17:12 ` Jani Nikula
2018-09-11 17:31 ` Mark Brown
2018-09-11 17:41 ` Daniel Vetter
2018-09-11 18:54 ` Mark Brown
2018-09-11 18:03 ` Geert Uytterhoeven
2018-09-11 17:22 ` James Bottomley
2018-09-11 17:56 ` Mark Brown
2018-09-11 18:00 ` James Bottomley
2018-09-11 18:16 ` Mark Brown
2018-09-11 18:07 ` Geert Uytterhoeven
2018-09-12 9:09 ` Dan Carpenter
2018-09-11 17:26 ` Mark Brown
2018-09-11 18:45 ` Steven Rostedt
2018-09-11 18:57 ` Daniel Vetter
2018-09-11 20:15 ` Thomas Gleixner
2018-09-12 9:03 ` Dan Carpenter
2018-09-10 23:01 ` Eduardo Valentin
2018-09-10 23:12 ` Steven Rostedt
2018-09-10 23:32 ` Eduardo Valentin
2018-09-10 23:38 ` Guenter Roeck
2018-09-10 23:38 ` Sasha Levin
2018-09-07 2:33 ` Steven Rostedt
2018-09-07 2:52 ` Guenter Roeck
2018-09-07 14:37 ` Laura Abbott
2018-09-07 15:06 ` Sasha Levin
2018-09-07 15:54 ` Laura Abbott
2018-09-07 16:09 ` Sasha Levin
2018-09-07 20:23 ` Greg KH
2018-09-07 21:13 ` Sasha Levin
2018-09-07 22:27 ` Linus Torvalds
2018-09-07 22:43 ` Guenter Roeck
2018-09-07 22:53 ` Linus Torvalds
2018-09-07 22:57 ` Sasha Levin
2018-09-07 23:52 ` Guenter Roeck
2018-09-08 16:33 ` Greg Kroah-Hartman
2018-09-08 18:35 ` Guenter Roeck
2018-09-10 13:47 ` Mark Brown
2018-09-09 4:36 ` Sasha Levin
2018-09-10 16:20 ` Dan Rue
2018-09-07 21:32 ` Dan Carpenter
2018-09-07 21:43 ` Sasha Levin
2018-09-08 13:20 ` Dan Carpenter
2018-09-10 8:23 ` Jan Kara
2018-09-10 7:53 ` Jan Kara
2018-09-07 3:38 ` Al Viro
2018-09-07 4:27 ` Theodore Y. Ts'o
2018-09-07 5:45 ` Stephen Rothwell
2018-09-07 9:13 ` Daniel Vetter
2018-09-07 11:32 ` Mark Brown
2018-09-07 21:06 ` Mauro Carvalho Chehab
2018-09-08 9:44 ` Laurent Pinchart
2018-09-08 11:48 ` Mauro Carvalho Chehab
2018-09-09 14:26 ` Laurent Pinchart
2018-09-10 22:14 ` Eduardo Valentin
2018-09-07 14:56 ` Sasha Levin
2018-09-07 15:07 ` Jens Axboe
2018-09-07 20:58 ` Mauro Carvalho Chehab
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=20180912131001.GG2760@piout.net \
--to=alexandre.belloni@bootlin.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=ksummit-discuss@lists.linuxfoundation.org \
--cc=laurent.pinchart@ideasonboard.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).