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

[Bekket McClane via llvm-dev]

Hi Alex,

Thank you very much for raising this crucial issue again.
And I think the review corner would be a good way to solve it.

However, I’m worry about that the traffic of the list would eventually become too high, and looses the ‘highlighter’ feature that it should have
After all, no one wants the ‘headline of unreviewed patches' to be a 1000+ items list. 

I suggest that we can aid this problem from another aspect at the same time: Notify the reviewer actively.
We can inference the candidate code owner/reviewer who is responsible for the patch from the submitted code 
And provide the submitter an option to notify those reviewers by email.

I think with the aforementioned approach, we can keep the review corner while preventing it to become another {llvm,cfe}-commits
Also, I think it can help new contributors with some problems related to code owners and code review assignees. For example, sometimes a new submitter either doesn't know who should be assigned for a code review, or the part he/she committed doesn’t seem to be covered by any code owner listed in CODE_OWNER.txt (e.g Transforms/Utils, IIRC).

Hope my opinion helps

Best Regards,
Bekket
> Alex Bradbury via llvm-dev <llvm-dev at lists.llvm.org> 於 2017年8月26日 下午7:01 寫道：
> 
> 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev