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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
