[llvm-dev] Phabricator sending spurious "This revision was not accepted when it landed" emails

[David Blaikie via llvm-dev]

+Mehdi AMINI <joker.eph at gmail.com> who's taking some (shared?) ownership of
Phabricator these days.

Mehdi - was Phab updated recently (such that we might've picked up new
semantics)?

On Tue, Jul 21, 2020 at 4:25 AM Jay Foad via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Has anyone else noticed Phabricator sending emails saying:
>   This revision was not accepted when it landed; it landed in state
> "Needs Review".
> when the review clearly has been accepted by someone?
>
> Some recent examples:
> https://reviews.llvm.org/D83952
> https://reviews.llvm.org/D80116


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

Do you have any examples that didn't have post-approval-pre-commit changes
that still got this annotation about being committed without review?

https://reviews.llvm.org/D81267


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.


>
>
> Thanks,
> Jay.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200721/f5208356/attachment-0001.html>