[PATCH] D80846: [llvm-ar] Update error messages and tests as per latest preferred style

Tue Jun 2 01:38:48 PDT 2020

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with the formatting changes undone, and the test changes rebased once the new tests from D80838 <https://reviews.llvm.org/D80838> are moved out.

================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:663
     if (sys::path::is_absolute(*FileNameOrErr)) {
-      NMOrErr->MemberName = Saver.save(sys::path::convert_to_slash(*FileNameOrErr));
+      NMOrErr->MemberName =
+          Saver.save(sys::path::convert_to_slash(*FileNameOrErr));
----------------
smeenai wrote:
> I'm guessing this was done by the linter as part of its formatting? It's a valid change, but it shouldn't be part of this diff.
> 
> Depending on how people who contribute to llvm-ar (e.g. @MaskRay) feel, we can either commit the formatting changes separately or just undo them (and ignore any lint warnings about them).
My personal take: if you are making changes in the immediate area (e.g. within a couple of lines), then it's okay to make the formatting change, either as part of, or in addition to the current patch, but that doesn't apply here, so I think this and the other formatting change below should be reverted.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80846