tfiala added a comment.

Okay I had a look now.  Just one comment on the code (inline), just for 
consideration.  Other than that, LGTM.


================
Comment at: packages/Python/lldbsuite/test/dotest_args.py:73
@@ -72,2 +72,3 @@
     group.add_argument('-s', metavar='name', help='Specify the name of the dir 
created to store the session files of tests with errored or failed status. If 
not specified, the test driver uses the timestamp as the session dir name')
+    group.add_argument('-S', metavar='format', help='Specify session file name 
format.  See configuration.py for a description.  Default="fnmac"')
     group.add_argument('-y', type=int, metavar='count', help="Specify the 
iteration count used to collect our benchmarks. An example is the number of 
times to do 'thread step-over' to measure stepping speed.")
----------------
> group.add_argument('-S', metavar='format', help='Specify session file name 
> format.  See configuration.py for a description.  Default="fnmac"')

Maybe include a long-style arg name as well?  Something like:

group.add_argument('--session-file-format', '-S', metavar='format', 
help='Specify session file name format.  See configuration.py for a 
description.  Default="fnmac"')

Then it could be referenced as "if args.session_file_format" instead of "if 
args.S".  But users could still use either '--session-file-format' or '-S' on 
the command line.

Not a big deal, but more readable for future maintainability, particularly if 
these options get ensconced in other CI scripts and the like, where a longer 
name is more helpful to see.


http://reviews.llvm.org/D20306



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to