[llvm-dev] [RFC] 'Review corner' section in LLVM Weekly

Alex Bradbury via llvm-dev llvm-dev at lists.llvm.org
Sat Aug 26 16:01:28 PDT 2017


Hi all. I'm assuming most people reading this email are familiar with LLVM's
code review process <http://llvm.org/docs/DeveloperPolicy.html#code-reviews>
as well as LLVM Weekly, the development newsletter I've written and sent out
every Monday since Jan 2014. Since that time, it's provided something of a
"signal boost" for important mailing list discussions and commits. I feel it
could play a similar role in helping patches that are stuck waiting for code
reviews, or drawing attention to submissions from first time contributors.
There may be alternative or complementary approaches to tackling this
perceived problem we should discuss - I'm coming from a position of trying to
apply the tools I have at my disposal. Also see my previous thoughts on this
issue <http://lists.llvm.org/pipermail/llvm-dev/2016-November/106696.html>.

I don't think it's controversial to suggest that while the code review process
works fantastically well a lot of the time, some patches fall through the
cracks and long delays in review feedback can put people off contributing to
LLVM. As was pointed out in response to the last RFC, very long review times
are problems for long-time contributors as well as newcomers.

My proposal is simple: add a new 'Review corner' section to LLVM Weekly to
help highlight patches that need more reviewer input. There are two main
categories I'd like to focus on:
1) patches from first-time contributors
2) patches where review activity has died off (i.e. they're 'stuck').

Obviously this is something that I can just go ahead and do, but I'd
appreciate feedback on whether this would be useful, as well as the specifics
of my proposed approach. I'd argue such a listing does provide value above and
beyond the firehose of {llvm,cfe}-commits, as the two mentioned categories are
not easily discoverable.

How to select the patches to include? I'm going to rule out manual curation -
LLVM Weekly is already a large time commitment and I'm not convinced trawling
llvm-commits is the way to about this even if time weren't a problem. Instead
I suggest that authors of patches that have got stuck in the review process
should submit their work for inclusion via a simple web form. If reviews have
stalled for a given period of time (1/2 weeks?), just submit a link to the
patch, and up to two sentences describing what the patch does and why people
might want to take a look at it. This might also be used to highlight patches
that aren't short of reviews but could benefit from a wider audience
(obviously if changes are large enough, an RFC should be submitted to
llvm-dev/cfe-dev as usual). The Phabricator API could be used to identify
patches from first time contributors for automatic inclusion, and ideally to
track the stats related to previously included patches (perhaps generate and
include credits for those who stepped up and helped review the previous week's
list?).

So, what do people think, is this worth a go? I recognise this proposal makes
a fairly large assumption, that lack of visibility is the problem to be
solved. If the underlying problem is that not enough people have the time or
willingness to review code, no amount of signal boosting is going to improve
things.

Thanks for reading,

Alex


More information about the llvm-dev mailing list