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

Alex Bradbury via llvm-dev llvm-dev at lists.llvm.org
Tue Aug 29 06:23:53 PDT 2017


On 29 August 2017 at 12:22, Florian Hahn <florian.hahn at arm.com> wrote:
> Hi Alex,
>
> On 27/08/2017 00:01, Alex Bradbury via llvm-dev wrote:
>> 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').
>>
>
> Thanks for your efforts to improve this process.
>
> I played around with the Phabricator API a while ago (I think after a post
> where you suggested improving the experience for first-time contributors)
> and built a small script that identifies first-time contributors and adds
> myself as subscriber to those reviews.
> I shared the script here https://reviews.llvm.org/D37259 .
> It would need some more work to turn it into a proper script, but I thought
> it would be a good time to share it now.

Thanks for sharing, this seems like a really useful starting point.
One potential direction would be a simple bot along the lines of the
rust-highfive which assigns at least an initial reviewer from a pool
of people hoping to help people get started contributing (see
https://github.com/rust-lang/rust/pull/44134#issuecomment-325454987
for an example). In the last RFC thread, I think Paul Robinson called
these 'first responders'. While on the subject of Phabricator bots,
having one which complains when a patch is posted without llvm-commits
or cfe-commits subscribed would probably help avoid patches falling
through the cracks due to a simple oversight.

> I think it should be relatively easy to find reviews where activity has died
> off using the API (excluding comments & patches submitted to llvm-commits
> directly).

I think using the API to find patches where activity has died off
might be useful for stats tracking. I'd rather rely on the patch
author choosing to make a submission for these stalled reviews though,
as I think part of the deal should be that the patch author asserts:
1) They're still interesting in landing the patch
2) They've ensured the patch applies cleanly on current LLVM/Clang
3) They've addressed any review comments they've received so far

Best,

Alex


More information about the llvm-dev mailing list