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

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

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

On 27 August 2017 at 01:50, Bekket McClane <bekket.mcclane at gmail.com>
wrote:
> 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.
Hi Bekket, thanks for the response.

I've been following llvm-commits pretty intently for the last 6 weeks
or so. My impression is that the list of patches that are stuck on
reviews and received a 'ping' in that time isn't too huge. I think
the
important thing to remember is that the review process is working well
for a large volume of patches, so we're talking about highlighting a
fairly small subset.
> 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).
Do you get the impression people have problems following the advice of
looking at the commit history for the files they're modifying and
adding reviewers based on that?

Best,

Alex

Hi Alex,

Thank you for trying to solve a very real problem.

I'm not sure this method will work -- but we'll never know without
trying.
Either way, your willingness to act and do something is very much
appreciated by me personally.

Yours,
Andrey
---
Compiler Architect
NXP

On Sunday, August 27, 2017, Alex Bradbury via llvm-dev <
llvm-dev at lists.llvm.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.
>
> Thanks for reading,
>
> Alex
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <javascript:;>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170828/fe61dcdf/attachment.html>

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>.
Hans Wennborg suggested on Twitter that bugs could also be included. I
can't write a coherent response in 140 characters, so am responding
here.

I think that highlighting important bugs is also a useful activity,
but I have less faith that there's something LLVM Weekly can do to
help here. There may be some value in highlighting bugs which are
release blockers (though I already try to make sure I link to any such
list when posted by the release manager) or a selection of 'beginner'
bugs, (props to Brian Gesiak for pushing for this category). But
beyond that, how do you decide which bugs to highlight? With patches,
it's normally the case that a highly motivated party (the patch
author) has put in the majority of the work, and a relatively smaller
amount of incremental effort is required from others in the LLVM
community (code review). With bugs it's the other way around - a huge
amount of additional work might be required to properly diagnose and
address a bug report.

I'm totally open to trying something if you think there's a way LLVM
Weekly can have an impact in this area, but I'm less hopeful about the
potential impact in reducing the number of open bugs.

Best,

Alex

Hi Alex, hi list,

I think this is a great idea.
I could suggest you setting up Google Forms[1] (or something similar). This way
you can give access to some volunteers (count me in) who can help you moderate
those submissions.

As a side note, Rust has a thing called “This week in Rust”[2], which is
basically a community moderated weekly. You may adopt some parts of this
approach if it sounds good to you.

[1] https://www.google.com/forms/about/
[2] https://this-week-in-rust.org
> On 27. Aug 2017, at 01:01, Alex Bradbury via llvm-dev <llvm-dev at
lists.llvm.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.
> 
> Thanks for reading,
> 
> Alex
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
--
AlexDenisov
Software Engineer, https://lowlevelbits.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: Message signed with OpenPGP
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170828/d0ec37e1/attachment.sig>

On Mon, Aug 28, 2017 at 8:04 AM, Alex Bradbury <asb at asbradbury.org>
wrote:
> 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>.
>
> Hans Wennborg suggested on Twitter that bugs could also be included. I
> can't write a coherent response in 140 characters, so am responding
> here.
The reason I brought it up is because I think the situation is
somewhat similar to stalled reviews; in fact forgotten bug reports are
even worse in a way: if no developers are cc'd on the bug, the pings
don't go anywhere -- no mailing list, no nothing. At least with code
reviews, pings usually go to one of the -commits lists.
> I think that highlighting important bugs is also a useful activity,
> but I have less faith that there's something LLVM Weekly can do to
> help here. There may be some value in highlighting bugs which are
> release blockers (though I already try to make sure I link to any such
> list when posted by the release manager) or a selection of
'beginner'
> bugs, (props to Brian Gesiak for pushing for this category). But
> beyond that, how do you decide which bugs to highlight? With patches,
> it's normally the case that a highly motivated party (the patch
> author) has put in the majority of the work, and a relatively smaller
> amount of incremental effort is required from others in the LLVM
> community (code review). With bugs it's the other way around - a huge
> amount of additional work might be required to properly diagnose and
> address a bug report.
>
> I'm totally open to trying something if you think there's a way
LLVM
> Weekly can have an impact in this area, but I'm less hopeful about the
> potential impact in reducing the number of open bugs.
What I was thinking when I replied to your tweet was something like
"bugs filed in the last 7 days which no-one seems to have looked at",
or something similar. Hopefully it should be possible to build the
list automatically.

I'm not sure how large that list would be each week though, or how
useful it would be. Also, this might be something we should fix by
having a better bug triaging process in general.

Cheers,
Hans

Hi Alex,

On 27/08/2017 00:01, Alex Bradbury via llvm-dev 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').
> 
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.

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).

Cheers,
Florian

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

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

On 18 September 2017 at 20:09, Alex Bradbury <asb at asbradbury.org>
wrote:
> 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.
As you've hopefully seen, the first iteration of the "Review
corner"
made its debut in Monday's LLVM Weekly:
http://llvmweekly.org/issue/195

Four 'stuck' reviews were submitted and since then, 3/4 have seen
fresh review activity - thanks to Dimitry Andric, Matthias Braun, and
Anna Zaks. So far so good, but lets try and make it 4/4 (see
https://reviews.llvm.org/D36357).

I haven't yet received any review submissions for next week, though
that may be because people prefer to wait until close to the deadline
(Sunday) to see if the review process moves forwards on its own. The
submission form is here http://llvmweekly.org/reviewcorner

Best,

Alex