[OPENMP] cbergstrom as OMP patch reviewer RFC

[Chandler Carruth]

On Fri, May 23, 2014 at 12: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.
>

I don't think that is a fair characterization of what I wrote.

First, I'm not objecting to review. Review is great, and more review from
more places is great.

I am saying that I don't think it is appropriate for someone who isn't a
regular contributor to Clang and familiar with the specific parts of Clang
(in the open, internal branches, etc. don't really count for open source
work) to sign off on a patch being committed to Clang.


>
> To further clarify - in some cases I'm acting as a proxy to other
> engineers on our team.
>

I don't think it is reasonable for you or anyone else to act as some form
of official proxy for the actual engineers. In the open source community,
we *need* to interact with the actual engineers directly in order to build
trust and working relationships with them. I don't see any evidence of that
being a workable strategy in LLVM certainly. Even when folks are
contributing patches originally authored by others, they both acknowledge
the authors of the work (where possible) and take full responsibility for
it going forward.

(Most of these engineers have many years of experience working with clang).
>

In the open source community? While I respect that many folks outside of
the open source community have expertise here, I don't think it is really
useful to insist that the open source community take that on faith with no
evidence. I also think it is specifically useful for the open source
community to encourage and incentivize people to contribute to the upstream
project.

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

A second set of eyes seems awesome. But we still need someone who works
with Clang in the open source project to review the patches and OK them for
submission.

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

I don't think this is either a concern or relevant to the issue.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140523/28b2b367/attachment.html>