[PATCH] D80846: [llvm-ar] Update error messages and tests as per latest preferred style
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
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
More information about the llvm-commits
mailing list