[Openmp-dev] [OPENMP] cbergstrom as OMP patch reviewer RFC

Andrey Bokhanko andreybokhanko at gmail.com
Thu May 22 23:36:54 PDT 2014


+1

Also, Chandler, is it safe to assume that if some time (a week? two weeks?)
passed and no-one but Chris (or someone else knowledgeable with OpenMP but
not clang code owner) reviewed a patch, the very fact of no review [from
clang code owners] suggests that the patch is not disruptive enough / too
OpenMP-specific for clang code owners to step in and do code review
personally? Especially given that said "clang code owners" are extremely
few in numbers -- like three people on Earth, if I remember correctly?

Granted, there are areas where a review from code owner is a *must*. But in
my opinion, such areas are quite limited in case of OpenMP patches --
basically, only when common infrastructure / code is changed. Agree?

Andrey



On Fri, May 23, 2014 at 10:00 AM, "C. Bergström"
<cbergstrom at pathscale.com>wrote:

> On 05/23/14 11:55 AM, Chandler Carruth wrote:
>
>> Unrelated to this specific patch, however:
>>
>> On Thu, May 15, 2014 at 4:17 AM, "C. Bergström" <cbergstrom at pathscale.com<mailto:
>> cbergstrom at pathscale.com>> wrote:
>>
>>     LGTM - maybe give it 1-2 more days for others to have a chance to
>>     review and if no objections push - Thanks
>>
>>
>> Please don't mark patches as looking good and encouraging the
>> contributors to commit for areas of the code and/or project you have no
>> familiarity with. I understand that you're quite familiar with OpenMP, but
>> as you aren't a regular Clang contributor (very few patches submitted, none
>> I could find committed directly) and aren't one of the maintainers or code
>> owners of Sema, it seems better to leave the final review to others.
>>
> I'm changing the subject to avoid hijacking the original thread
> ----------
> Chandler has raised concerns I'm unqualified to review any of the OpenMP
> work being submitted by Intel. I respect his opinion, but at the same time
> I'd like to continue reviewing these patches where I have the time. I
> obviously can't do this unless others feel I am qualified though.
>
> To further clarify - in some cases I'm acting as a proxy to other
> engineers on our team. (Most of these engineers have many years of
> experience working with clang). I realize bandwidth for the clang
> developers to review OMP is limited - my goal is to help provide a 2nd set
> of eyes on low risk contributions in order to help get great OMP support in
> clang. You'll see based on history I don't review random things.
>
> 2nd - in the event my review isn't perfect and there's some intricate
> detail I missed - in general the Intel team is extremely responsive and a
> minor follow-up commit would likely fix any nit. (With the downside of some
> commit noise, but in general I don't think every llvm/clang commit is
> perfect out of the box - I can't speak for the community, but it seems
> reasonable to me)
>
> If anyone wants to +1 me as a reviewer for the OMP work please speak up.
> (Otherwise I won't be able to provide further help in this area)
>
> Thanks
>
> _______________________________________________
> Openmp-dev mailing list
> Openmp-dev at dcs-maillist2.engr.illinois.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/openmp-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140523/7dd456f1/attachment.html>


More information about the cfe-commits mailing list