<div dir="ltr"><div class="gmail_quote"><div>Hi Aaron,</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> “Code reviews are conducted on our web-based code-review tool (see Code Reviews with Phabricator).”<br>
<br>
Personally, I am in favor of this policy for initiating code reviews,<br>
but am opposed to it for post-commit feedback. The problem, as I see<br>
it, is that not every change *gets* code review via Phab and the email<br>
lists are the only place on which to provide that post-commit<br>
feedback. </blockquote><div><br></div><div>You can set up notifications on commits in Phabricator. Phabricator adds the user "llvm-commits" as subscriber to certain reviews: <a href="https://reviews.llvm.org/H615" target="_blank">https://reviews.llvm.org/H615</a></div><div>We could do the same thing for commits...</div><div><br></div><div>So you can reply to any commit via the web UI (or email notification) and the author gets notified as well. One example that worked for me:</div><div> <a href="https://reviews.llvm.org/rG7f9717b922d421c30f889034488563c67076aca1" target="_blank">https://reviews.llvm.org/rG7f9717b922d421c30f889034488563c67076aca1</a><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The Phab commit message does not have any subscribers though, and so<br>
my understanding is that comments on that Phab interface are not<br>
communicated out to the community as a whole, which means the<br>
community may miss out on important post-commit-review information<br>
like general awareness of the problem, workarounds people can use<br>
until the author corrects something, alternative ideas on how to fix<br>
the issue, etc. Or do I misunderstand the way Phab works in this<br>
workflow?<br></blockquote><div><br></div><div>We can add automatically subscribers to commits as well if we want to. </div><div>One random example where a subscriber was added to a commit via a Herald rule:</div><div><a href="https://reviews.llvm.org/rG93537fabcee8fcfa3316d7abd5e935f7fe9b468f" target="_blank">https://reviews.llvm.org/rG93537fabcee8fcfa3316d7abd5e935f7fe9b468f</a><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Also, "the commenter would need to find the Phabricator commit"<br>
concerns me -- adding extra burden on the people providing feedback<br>
means that *some* amount of those people will struggle enough to<br>
simply not provide that feedback. Responding to an email is about as<br>
low as I think we can set that bar, so the current approach has better<br>
ergonomics for giving feedback. </blockquote><div><br></div><div>If we set up the notifications via Phabricator, you can reply via email. These contain a link at the bottom that will take you directly to the commit page in Phabricator. Not sure why we don't have these on the mailing list...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Tangential complaint -- our use of Herald causes some pain for me as a<br>
list moderator because we've reached the point where Herald<br>
automatically adds so many subscribers to a review that it gets marked<br>
as spam for every email that is generated for the review. <br></blockquote><div><br></div><div>So this would be a reason to replace the XXX-commits mailing lists with something else...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Given how often Phabricator goes down (which is not super often, but<br>
often enough that people know it happens), I am deeply uncomfortable<br>
with the idea of shutting down the current *-commits mailing lists<br>
because of the chance for data loss. Personally, I think the *-commits<br>
lists are working well and I would prefer they be left alone.<br></blockquote><div><br></div><div>If Phabricator is down, you're not getting any email notifications from it anyway. So we might already be losing data right now...</div><div><br></div><div>But fair point: The more we rely on Phabricator, the higher the reliability requirements.</div><div><br></div><div>Best,</div><div>Christian</div><div><br></div><div><br></div></div></div>