<div dir="ltr"><div><a class="gmail_plusreply" id="plusReplyChip-0" href="mailto:joker.eph@gmail.com" tabindex="-1">+Mehdi AMINI</a> who's taking some (shared?) ownership of Phabricator these days.<br><br>Mehdi - was Phab updated recently (such that we might've picked up new semantics)?</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jul 21, 2020 at 4:25 AM Jay Foad via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Has anyone else noticed Phabricator sending emails saying:<br>
  This revision was not accepted when it landed; it landed in state<br>
"Needs Review".<br>
when the review clearly has been accepted by someone?<br>
<br>
Some recent examples:<br>
<a href="https://reviews.llvm.org/D83952" rel="noreferrer" target="_blank">https://reviews.llvm.org/D83952</a><br>
<a href="https://reviews.llvm.org/D80116" rel="noreferrer" target="_blank">https://reviews.llvm.org/D80116</a></blockquote><div><br>Hard for me to tell what happened here. I wonder if it's related to making changes after review/before committing. While that's common in LLVM, I could imagine a review tool (especially if we picked up a newer version - as I don't think it's always had this behavior) might get fussy about that - perhaps it'd be configurable, so it'd say "this was committed with extra changes" but not "This was committed without review".<br><br>Do you have any examples that didn't have post-approval-pre-commit changes that still got this annotation about being committed without review?<br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<a href="https://reviews.llvm.org/D81267" rel="noreferrer" target="_blank">https://reviews.llvm.org/D81267</a></blockquote><div><br></div><div>Last one seems more clear - one of the reviewers (rupprecht) still had the review marked "requires changes", so it was committed without closure on that.</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"><br>
<br>
Thanks,<br>
Jay.<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>