[PATCH] D107695: [llvm] [lit] Support forcing lexical test order

[Julian Lettner via Phabricator via llvm-commits]

yln added inline comments.


================
Comment at: llvm/utils/lit/lit/cl_arguments.py:154
             type=_positive_int)
+    selection_group.add_argument("--default-order",
+            dest="order",
----------------
yln wrote:
> jdenny wrote:
> > Instead of `--default-order`, I'd prefer something that describes the order.  I'm not sure of the best name, especially given that it's always possible we'll think of other tweaks to the default order in the future.  Perhaps something like `--auto-order` or `--smart-order` to at least suggest it's something clever.  What do you think?
> I would also prefer descriptive names instead of "default".
> 
> If we already put the effort i to improve ordering, how about: `--order=(aaa|bbb|...)` and make the old ones aliases for the new ones: `--shuffle` --> `--order=random`.
> 
> I think argparse spells out all possible options and even shows which one is the default when printing the help: `--order=(smart|aaa|bbb|...) [default: smart]`
> 
> So it shows the user the available options, which option is the default, and the explanation of these options is right next to it.
> Having all options documented right next to each other also improves discoverability and makes contrasting the differences between options easier.
If it wasn't clear, I am advocating for using `parser.add_argument('--order', choices=['smart', aaa', 'bbb'])` here.
https://docs.python.org/3/library/argparse.html#choices


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

https://reviews.llvm.org/D107695