[PATCH] D83088: Introduce CfgTraits abstraction

Fri Oct 23 08:19:06 PDT 2020

arsenm added a comment.

In D83088#2348641 <https://reviews.llvm.org/D83088#2348641>, @mehdi_amini wrote:

> In D83088#2347111 <https://reviews.llvm.org/D83088#2347111>, @arsenm wrote:
>
>> In D83088#2346322 <https://reviews.llvm.org/D83088#2346322>, @mehdi_amini wrote:
>>
>>> In D83088#2345540 <https://reviews.llvm.org/D83088#2345540>, @nhaehnle wrote:
>>>
>>>> David, I don't think this is appropriate here. Let's take the discussion to llvm-dev.
>>>
>>> Seems like David asked to revert in the meantime?
>>
>> -1 to reverting, which will just make the history messier with no tangible benefit
>
> This is the usual LLVM policy I believe: someone expressed a concern and ask to revert. We revert and discuss first.
> So again: please revert.
>
> The messier history is not an argument: we revert so many times for any random bot failures already, and our contribution guidelines still tell people to push a "fake commit" with a whitespace change to test their access.

Unrelated, but I think the test commit process should be dropped

> I also see tangile benefits:
>
> - we don't start building dependencies on newly introduced API making a revert more difficult later.
> - the burden of convincing of the approach is on the patch author, reverting is forcing the discussion here.

This patch has been up for review for almost 4 months, with a corresponding RFC on llvm-dev. The last review comments were over 2 months ago. Coming back to this so long after to ask for a revert is an unworkable level of review paralysis

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83088/new/

https://reviews.llvm.org/D83088