[PATCH] D67558: [llvm-ar] Uncapitalize and delete full stop from error messages

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 13 09:35:18 PDT 2019


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

LGTM with a nits.



================
Comment at: tools/llvm-ar/llvm-ar.cpp:386
     if (!Members.empty())
-      fail("The s operation takes only an archive as argument");
+      fail("the s operation takes only an archive as argument");
   }
----------------
MaskRay wrote:
> grimar wrote:
> > btw, below modifiers are sometimes wrapped into `''`. May be worth to make this consistent in messages while you are here too?
> Wrapped in `''` for consistency.
> 
> It seems many error messages here are no tested. This is something we should improve in the future.
Yeah. I also found one more inconsistency:

"cannot mix -M and other options"

Seems in other places we use the word "operation", but here the word "option" with a dash.
It is anyways out of the area of this patch, just had to mention :)


================
Comment at: tools/llvm-ar/llvm-ar.cpp:212
   if (PositionalArgs.empty())
-    fail("Expected [relpos] for a, b, or i modifier");
+    fail("expected [relpos] for a, b, or i modifier");
   RelPos = PositionalArgs[0];
----------------
nit: needs wrapping too it seems.


================
Comment at: tools/llvm-ar/llvm-ar.cpp:221
   if (PositionalArgs.empty())
-    fail("Expected [count] for N modifier");
+    fail("expected [count] for N modifier");
   auto CountParamArg = StringRef(PositionalArgs[0]);
----------------
The same.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67558





More information about the llvm-commits mailing list