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

Bekket McClane via llvm-dev llvm-dev at lists.llvm.org
Sun Aug 27 07:20:24 PDT 2017


> Alex Bradbury <asb at asbradbury.org> 於 2017年8月27日 上午6:43 寫道:
> 
> 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.

Ah, that’s great.

> 
>> 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?

Sometimes contributors in the commit history are not able to review, or if they’re not the code owner, they’re not mandatory to take responsibility for reviewing the patch.
Will, I think this problem would somehow lead back to the original one, where there are patches left unreviewed though they’re assigned to somebody.
But I think finding the right people to review is still an important issue. 

Regards,
Bekket

> 
> Best,
> 
> Alex



More information about the llvm-dev mailing list