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

[Alex Bradbury via llvm-dev]

On 27 August 2017 at 00:01, Alex Bradbury <asb at asbradbury.org> wrote:
> 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.

Seeing as the idea got positive feedback, I'm going to trial it
starting from next week. Please submit patches which are awaiting
review here <http://llvmweekly.org/reviewcorner>. Let's start simple
and see if it works. I've suggested the following guidelines for
submitting a code review thread for inclusion:
* The patch has gone at least two weeks without substantial review
feedback OR this is your first patch to an LLVM project
* You have updated the patch based on any review comments received so far
* The patch has been compile tested against the current HEAD of the
appropriate LLVM project, and rebased if necessary
* You are willing to write a short two-sentence summary that explains
1) what the patch does, and 2) why people might interested.

Best,

Alex