<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jun 25, 2014 at 12:32 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
On 25/06/2014 21:18, Eli Bendersky wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
<br>
<br>
<br>
On Wed, Jun 25, 2014 at 11:10 AM, Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a> <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>> wrote:<br>
<br>
<br>
    On 25/06/2014 21:03, Eli Bendersky wrote:<br>
<br>
        On Wed, Jun 25, 2014 at 10:44 AM, Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><br></div>
        <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>> <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><div><div class="h5"><br>
        <mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>>> wrote:<br>
<br>
            For whatever reason, patches posted to the Phabricator website<br>
            still aren't being sent to the mailing list, making it<br>
        difficult<br>
            for us to review them.<br>
<br>
            I've raised this issue a couple of times in the last few<br>
        weeks.<br>
<br>
            In practice this has a detrimental effect to the development<br>
            workflow because it means that code is being seen only by<br>
        a small<br>
            group of individuals who have web accounts. The code isn't<br>
        hitting<br>
            llvm-commits or cfe-commits where the majority of code<br>
        maintainers<br>
            use the mailing lists for review.<br>
<br>
            At this point I think Phabricator should be disabled and<br>
        patches<br>
            should be send to the mailing lists *until* the technical<br>
        issue is<br>
            confirmed resolved.<br>
<br>
            It's really uncool that code is entering ToT through this<br>
            back-channel -- I appreciate that it might not be<br>
        intentional, but<br>
            every single patch that gets committed this way is a real<br>
        problem<br>
            for the project.<br>
<br>
<br>
        Phabricator has certainly had its share of technical<br>
        difficulties lately. Just last week it suppressed all email to<br>
        llvm-commits for many hours. These problems should be solved.<br>
        That said, talking of "private reviews" and "back-channels"<br>
        doesn't strike me as constructive.<br>
<br>
<br>
    Eli, I wasn't making a value judgement. That's exactly what they are:<br>
<br>
      1) They're private reviews because they're conducted away from<br>
    the LLVM community.<br>
      2) It's a back-channel because the only means of veto is to<br>
    revert the patch or attempt to "fix forward" post-commit.<br>
<br>
    I already pointed out that it may not be intentional -- <br>
<br>
"May" not be intentional suggests that it also "may" be intentional. Or is it English comprehension failing me? [sorry, 3rd language...]<br>
</div></div></blockquote>
<br>
Hi Eli,<br>
<br>
The sentence is definitely written as intended and it's that way in order to avoid making value judgements.<br>
<br>
I don't have any evidence whether the public lists have been excluded intentionally or due to technical issues so I can't say "is" or "isn't".<br>
<br>
As I understand, some people legitimately use Phabricator for internal review, while others *think* they're submitting public patches but the system doesn't forward them to the reviews lists so my usage of "may" is entirely correct.<div class="">

<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
When llvm-commits is CCd on the Phabricator review, any suggestion of intentional hiding is not only inappropriate, but also somewhat ridiculous. Unless you're suggesting someone is planting those PHP bugs? [that could be unintentional, I concede, since PHP is just one big bug in general]<br>


</blockquote>
<br></div>
Clearly I'm not suggesting that someone is planting PHP bugs :-)<br>
<br>
But yes, you can probably tell I'm disappointed that it's still happening, because ultimately I care about the quality of the code and delivering a great compiler to our users. So how about we get to the bottom of these issues?</blockquote>

<div><br></div><div>Right. It would be useful to see actual examples and discuss them on a case-by-case basis. I imagine you sent the original email when observing such a case (or a few) - could you share the review link? I think this thread discussed a number of ways in which such things could go wrong, and it would be interesting to see which of those applies to the example you're presenting.</div>

<div><br></div><div>Eli</div><div><br></div><div> </div></div></div></div>