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

Sun Apr 7 13:55:00 PDT 2019

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.

Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D60351