[llvm-dev] What to do when a developer ignores post-commit review comments

[David Blaikie via llvm-dev]

+Konstantin (patch author) and Matt (patch approver) for visibility.

Sometimes folks miss the signal in the noise of the -commits lists,
unfortunately - so probably an email here on the list (at least if the
breakage isn't totally obvious/failing buildbots/etc and noticed
relatively quickly (the longer a patch has been in, the higher the bar
to revert it, generally)), such as this one is probably a good start.
I'd give this thread a day or two to seek if the original author and
reviewer chime in.

Has the bug in the patch been fixed? Are there outstanding functional
issues with the patch as-is?

If the known bug is fixed and there aren't known functional issues
outstanding or fundamental design issues (see, for instance, the
CfgTraits ("RFC: CfgTraits/CfgInterface design discussion") episode
last year), it may not meet the bar for a revert at this point -
though some fairly firm requests for better test coverage could be
warranted. If the author continues not to respond to even this email,
I could see this maybe escalating to a revert - though that's super
rare and I don't think there's a general policy guideline that you
should take away from this thread for that situation - basically in
that situation, having this sort of thread is probably necessary to
gut-check/seek some agreement that it's enough of a problem to revert.

- Dave

On Mon, Apr 26, 2021 at 5:58 AM James Henderson via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> Hi all,
>
> TL;DR - what should you do if a patch author fails to respond to post-commit review comments? I'm guessing the answer is revert after sufficient prompting, but how much is sufficient prompting?
>
> Background:
>
> I was not too long ago involved in reviewing a patch on Phabricator (https://reviews.llvm.org/D95638). I made several comments, and some of these were addressed. Unfortunately, I didn't have the time to come back to the review at that time, and one or two others made a couple of review comments themselves. Eventually, after a couple of pings by the patch author, one of the people on the review approved the patch, and it was committed. About a week later, one of my colleagues noticed a bug in the patch, which prompted me to take another look at it. At this point, I noticed that the patch had landed without several of my comments being addressed; the patch in particular had (in my opinion) insufficient testing. I posted these comments on the Phabricator review page for the patch, asking for them to be addressed. After taking time off for a couple of weeks, I noticed that the patch author hadn't responded to my comments, so I prodded them again. A week later and there's still been no response.
>
> What should I do at this point? I can of course revert the patch (and any subsequent patch that relied on it). Before taking that action though, I wanted to make sure this was the appropriate approach.
>
> Thanks in advance for advice.
>
> James
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev