[PATCH] D93938: [clang-format] Fixed AfterEnum handling

Wed Mar 10 11:56:30 PST 2021

HazardyKnusperkeks added a comment.

In D93938#2616044 <https://reviews.llvm.org/D93938#2616044>, @atirit wrote:

> In D93938#2614825 <https://reviews.llvm.org/D93938#2614825>, @HazardyKnusperkeks wrote:
>
>> If the bugs are (very) similar, I could live with one fix for both. Otherwise you should fix the other bug first, if its blocking this change.
>
> The only thing linking the bugs is `AfterEnum`. Other than that I'd say they're separate. If the bug should be fixed first (instead of merging incorrect but passing tests) then I think it should be a separate diff.
>
> My concern with that course of action is that whoever reviews that diff will all but certainly ask for unit tests, and if I wrote unit tests for `AfterEnum` and `AllowShortCaseLabelsOnASingleLine` as I have here, they'll still be incorrect because this bug is not merged. Would you foresee this being an issue?
>
> I'll start working on a fix regardless.

Than I would say it should be one change. Omitting a (needed) test just so it passes is not good, and failing tests are worse. Just fix both at the same time.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938