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

[David Blaikie via llvm-dev]

On Tue, Jul 21, 2020 at 11:29 AM Mehdi AMINI <joker.eph at gmail.com> wrote:

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

In my inbox I have two emails for that review.
(though, also, on the commits list, I do see this:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200720/808735.html -
though my mail client (gmail)  is rendering this one as in the same
thread/doesn't seem to show the "[Differential]" prefix in the subject -
not sure what's going on there, usually gmail is /too/ ready to group mails
by actual subject text, rather than by thread ids in the email hedaers... )


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

Similarly, the separate "[Differential]" email seems to be unthreaded and
contains the problematic "not accepted when it landed" text:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200720/808779.html


>
>
>
>>
>> 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/3e065caf/attachment.html>