[PATCH] D60351: [builtins] Reformat builtins with clang-format

Eric Christopher via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 7 22:27:54 PDT 2019


On Sun, Apr 7, 2019 at 1:55 PM JF Bastien via Phabricator
<reviews at reviews.llvm.org> wrote:
>
> jfb added a comment.
>
> In D60351#1457640 <https://reviews.llvm.org/D60351#1457640>, @thakis wrote:
>
> > In D60351#1457455 <https://reviews.llvm.org/D60351#1457455>, @jfb wrote:
> >
> > > I'd much rather see format updates happen as the files are touched, instead of all-out like this, because it makes mergebots sad.
> >
> >
> > Huh, usually the guidance is "land reformat in a separate commit". And we've mass-reformated code that didn't follow LLVM style before, e.g. lldb. What's different here?
>
>
> Do you intend to change all these files in the near future? If we want to go by guidance:
>
> > "Avoid committing formatting- or whitespace-only changes outside of code you plan to make subsequent changes to. Also, try to separate formatting or whitespace changes from functional changes, either by correcting the format first (ideally) or afterward. Such changes should be highly localized and the commit message should clearly state that the commit is not intended to change functionality, usually by stating it is NFC." from https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
>
> Agreed that format changes should happen as NFC commits, but I'm just not a fan of commits that just do that because the code is "wrong". Unless there's subsequent changes coming to each file, it's useless churn that hurts mergebots and makes the history annoying to follow.
>
> Not that this is "over my dead body". I'm just asking if the motivation is just "oh hey it's wrongly formatted, let's fix the format". If so, I'm not a fan. I'd much rather fix code as we change it.
>
>

FWIW for me that's fine in cases where most of the files are in llvm
style and format, but like lldb, compiler-rt isn't really fabulous
about following the general style guidelines of the project.


More information about the llvm-commits mailing list