[PATCH] D35333: Create empty shell of llvm-mt.

[Eric Beckmann via Phabricator via llvm-commits]

ecbeckmann marked an inline comment as done.
ecbeckmann added a comment.

In https://reviews.llvm.org/D35333#810083, @ruiu wrote:

> Is all uppercase really a convention? I took a quick look at a few .td files in clang, and looks like they write option ids in lowercase.


hmm it seems the convention is capital first letter, rest lower case



================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:48
+               HELPTEXT, METAVAR, VALUES)                                      \
+  {                                                                            \
+      PREFIX,      NAME,      HELPTEXT,                                        \
----------------
ruiu wrote:
> ruiu wrote:
> > This open parenthesis is not at the same indentation depth as the closing one.
> Is this done?
shoot sorry for some reason git format keeps changing this.


================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:66
+LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg) {
+  errs() << "llvm-mt error: " << Msg;
+  exit(1);
----------------
ruiu wrote:
> ruiu wrote:
> > It is convenient to write out "\n" here, instead of embedding "\n" as part of Msg.
> It's not done.
hmmm in most recent revision it should be done


================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:71
+
+int main(int argc_, const char *argv_[]) {
+  sys::PrintStackTraceOnErrorSignal(argv_[0]);
----------------
ruiu wrote:
> ruiu wrote:
> > Please follow the LLVM naming convention.
> `argc` and `argv` still don't follow the LLVM naming convention.
on most recent revision this should be fine


https://reviews.llvm.org/D35333