From: Vegard Nossum <vegard.nossum@oracle.com>
To: Leon Romanovsky <leon@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jakub Kicinski <kuba@kernel.org>,
Linus Walleij <linus.walleij@linaro.org>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Andrew Lunn <andrew@lunn.ch>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Luis Chamberlain <mcgrof@kernel.org>,
Josef Bacik <josef@toxicpanda.com>,
ksummit@lists.linux.dev, Jeff Layton <jlayton@kernel.org>,
Song Liu <song@kernel.org>
Subject: Re: [MAINTAINERS SUMMIT] Maintainer burnout
Date: Mon, 21 Aug 2023 21:23:18 +0200 [thread overview]
Message-ID: <e7554530-a1a5-216f-9a17-7cf763ee6a9d@oracle.com> (raw)
In-Reply-To: <20230819064537.GM22185@unreal>
On 8/19/23 08:45, Leon Romanovsky wrote:
> It is worth to try to get honest feedback from active developers/contributors/vendors
> what is their "real" excuse for not doing code review.
>
> I saw in this thread about "have no time to do code review" answers and we
> all can relate to it, but IMHO it is just an excuse and not the real reason.
> Especially for a people who are employed by big corporations to do their
> upstream work.
Hi,
For some drive-by or would-be reviewers, at least, I think part of the
problem is perverse or misaligned incentives.
If you write code and your patches are accepted in the kernel, it counts
towards your commit count, which is a metric that people look at, for
better or worse (probably worse).
When you review a patch and you find some problem with it, the patch
will NOT get accepted in the kernel (at least not in that form), and
your name will NOT appear in the git log. So in a way, in order for
your contribution to get recorded, you have to give the patch a
passing grade -OR- you are now on the hook to keep reviewing every
following iteration of the patch until it's in a state where you're
completely sure it doesn't have any problems.
(Of course, if you just rubber-stamp your Reviewed-by: on everything
then you are bound to be found out sooner or later -- or at the very
least seen as an unreliable reviewer.)
But let's assume you don't give out your Reviewed-by: without having
REALLY checked the patch thoroughly. Even then, mistakes can slip in.
In a way, being a reviewer and missing something critical is even
worse than being the author and missing something critical. Is it even
worth putting your Reviewed-by: on something if you're not 100% sure
this patch is not going to cause an issue? Are people going to trust
you in the future if you make a mistake in your review?
Let's say you're completely sure you found an issue with the patch. Why
not just stay silent, hope that nobody catches it, and then submit your
own patch later to fix it? That will get your name in the log. Even
worse, if it's a security issue you can maybe write an exploit for it
and either get a bounty from Google or sell it for serious $$$ to
various malicious actors. [Note that I'm not saying most people would do
this; I don't know. I am just using it as an example to show that the
incentives are disproportionate.]
The incentives that remain (as far as I can tell) are:
1) you get familiar with a specific part of the kernel, and
2) you get goodwill and recognition from other kernel developers.
Both assume a certain volume of reviews to pay off in any significant
way. Even then, depending on your ultimate goals, doing reviews is
typically a low-ROI activity for individuals compared to things like
actually writing code and submitting patches.
If the community truly values reviews, then I think it's necessary to
find better mechanisms for recognizing and rewarding reviews.
Can we maybe adjust the standards of the Reviewed-by: tag to mean that
the person has looked at the patch (or an earlier version of it) and
provided comments that show they actually put some effort into it?
For example, if a patch is changing a function (as patches often do),
the reviewer should add a line saying "error paths in foo() lgtm"
and not just tack on their Reviewed-by: line.
This adjustment would make it harder to just slap a Reviewed-by: on a
patch, but it would also make it easier to get your name in the
changelog provided that you actually put the effort in.
Maybe release notes/shortlogs could also include reviewer stats as a
bonus. I'm aware of the regular LWN articles covering reviewer stats,
but again, I think that only includes reviews for patches that actually
passed the review and made it into the kernel.
I suspect that review duty often falls on maintainers because they know
their own subsystem the best (and maybe also because they are the
reviewer of last resort). I think that reviews are a great way to learn
more about a subsystem and we should encourage newcomers and non-experts
to contribute their reviews (and get credit for it) even if those
reviews are not as complete as a maintainer's or subsystem expert's
review would be. And we can encourage that by nudging the incentives
in the right direction.
Vegard
next prev parent reply other threads:[~2023-08-21 19:24 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-16 18:08 [MAINTAINERS SUMMIT] Maintainer burnout Josef Bacik
2023-08-16 20:14 ` Luis Chamberlain
2023-08-17 9:39 ` Laurent Pinchart
2023-08-17 12:36 ` Andrew Lunn
2023-08-17 15:19 ` Jakub Kicinski
2023-08-17 23:54 ` Alexei Starovoitov
2023-08-18 13:55 ` Linus Walleij
2023-08-18 15:09 ` Jakub Kicinski
2023-08-18 17:07 ` Linus Torvalds
2023-08-19 6:45 ` Leon Romanovsky
2023-08-21 15:35 ` Laurent Pinchart
2023-08-22 7:41 ` Jiri Kosina
2023-08-22 9:05 ` Hannes Reinecke
2023-08-22 10:13 ` Leon Romanovsky
2023-08-22 11:25 ` Laurent Pinchart
2023-08-21 19:23 ` Vegard Nossum [this message]
2023-08-22 4:07 ` Dave Airlie
2023-08-22 9:46 ` Jan Kara
2023-08-22 10:10 ` Christian Brauner
2023-08-22 10:20 ` Jan Kara
2023-08-22 11:29 ` Laurent Pinchart
2023-08-22 11:05 ` Leon Romanovsky
2023-08-22 11:32 ` Laurent Pinchart
2023-08-22 13:47 ` Leon Romanovsky
2023-08-22 13:30 ` Jan Kara
2023-08-29 12:54 ` Steven Rostedt
2023-09-13 9:02 ` Dan Carpenter
2023-08-21 8:50 ` Daniel Vetter
2023-08-21 15:18 ` Jakub Kicinski
2023-08-22 4:12 ` Dave Airlie
2023-08-18 15:26 ` Laurent Pinchart
2023-08-18 15:40 ` Konrad Rzeszutek Wilk
2023-08-18 18:36 ` Mark Brown
2023-08-21 16:13 ` Laurent Pinchart
2023-08-18 16:10 ` Mark Brown
2023-08-21 16:04 ` Laurent Pinchart
2023-08-24 21:30 ` Jonathan Cameron
2023-08-25 7:05 ` Krzysztof Kozlowski
2023-08-17 12:00 ` Jani Nikula
2023-08-17 12:17 ` Mark Brown
2023-08-17 12:42 ` Laurent Pinchart
2023-08-17 13:56 ` Miguel Ojeda
2023-08-17 15:03 ` Laurent Pinchart
2023-08-17 17:41 ` Miguel Ojeda
2023-08-18 15:30 ` Laurent Pinchart
2023-08-18 16:23 ` Mark Brown
2023-08-18 17:17 ` Laurent Pinchart
2023-08-18 18:00 ` Mark Brown
2023-08-17 14:46 ` Mark Brown
2023-08-17 14:22 ` Steven Rostedt
2023-08-17 15:31 ` Jani Nikula
2023-08-17 14:46 ` Steven Rostedt
2023-08-17 15:33 ` Josef Bacik
2023-08-17 17:10 ` Rodrigo Vivi
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=e7554530-a1a5-216f-9a17-7cf763ee6a9d@oracle.com \
--to=vegard.nossum@oracle.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrew@lunn.ch \
--cc=jlayton@kernel.org \
--cc=josef@toxicpanda.com \
--cc=ksummit@lists.linux.dev \
--cc=kuba@kernel.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=leon@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=mcgrof@kernel.org \
--cc=song@kernel.org \
--cc=torvalds@linux-foundation.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).