<div dir="ltr"><div><div>+1<br><br></div>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?<br>
<br></div><div>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?<br>
</div><div><br></div>Andrey<br><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, May 23, 2014 at 10: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">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>

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

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

<br>
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)<br>
<br>
Thanks<br>
<br>
______________________________<u></u>_________________<br>
Openmp-dev mailing list<br>
<a href="mailto:Openmp-dev@dcs-maillist2.engr.illinois.edu" target="_blank">Openmp-dev@dcs-maillist2.engr.<u></u>illinois.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/openmp-dev" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/openmp-dev</a><br>
</blockquote></div><br></div>