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

[Mehdi AMINI via llvm-dev]

On Tue, Jul 21, 2020 at 11:07 AM David Blaikie <dblaikie at gmail.com> wrote:

> +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)?
>

No: I upgraded the hardware and the OS, but not Phab itself yet.

I have a test instance running with an upgraded Phab though, it may have
been sending duplicate emails in the last day or two when I didn't notice I
had the email daemon running.


>
> 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
>
>
Seems like this one closed as expected without the message?
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200720/808734.html



>
>> https://reviews.llvm.org/D80116
>
>
Same here:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200720/808778.html

Can you forward me the email you received for these revisions?



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

Indeed this one shows the message:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200713/807554.html

-- 
Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200721/4e13aeea/attachment.html>