[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:42 AM David Blaikie <dblaikie at gmail.com> wrote:

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

Thanks! I dug a bit and found out that these came from my test instance, I
ran an upgraded Phabricator instance and didn't disable polling the
repository: so it closed the revision without having any approval.

-- 
Mehdi



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