[llvm-dev] [RFC] 'Review corner' section in LLVM Weekly (original) (raw)

Alex Bradbury via llvm-dev llvm-dev at lists.llvm.org
Mon Sep 18 12:09:04 PDT 2017

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:

  1. what the patch does, and 2) why people might interested.

Best,

Alex

More information about the llvm-dev mailing list