<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 23, 2014 at 12:00 AM, "C. Bergström" <span dir="ltr"><<a href="mailto:cbergstrom@pathscale.com" target="_blank">cbergstrom@pathscale.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":6hx" class="a3s" style="overflow:hidden">On 05/23/14 11:55 AM, Chandler Carruth wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Unrelated to this specific patch, however:<br>
<br>
On Thu, May 15, 2014 at 4:17 AM, "C. Bergström" <<a href="mailto:cbergstrom@pathscale.com" target="_blank">cbergstrom@pathscale.com</a> <mailto:<a href="mailto:cbergstrom@pathscale.com" target="_blank">cbergstrom@pathscale.<u></u>com</a>>> wrote:<br>

<br>
    LGTM - maybe give it 1-2 more days for others to have a chance to<br>
    review and if no objections push - Thanks<br>
<br>
<br>
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.<br>

</blockquote>
I'm changing the subject to avoid hijacking the original thread<br>
----------<br>
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.<br>
</div></blockquote><div><br></div><div>I don't think that is a fair characterization of what I wrote.</div><div><br></div><div>First, I'm not objecting to review. Review is great, and more review from more places is great.</div>
<div><br></div><div>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. </div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":6hx" class="a3s" style="overflow:hidden">
<br>
To further clarify - in some cases I'm acting as a proxy to other engineers on our team.</div></blockquote><div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":6hx" class="a3s" style="overflow:hidden"> (Most of these engineers have many years of experience working with clang).</div>
</blockquote><div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":6hx" class="a3s" style="overflow:hidden"> 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.</div>
</blockquote><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div id=":6hx" class="a3s" style="overflow:hidden">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)<br>
</div></blockquote><div><br></div><div>I don't think this is either a concern or relevant to the issue.</div></div></div></div>