-
-
Notifications
You must be signed in to change notification settings - Fork 151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Group CLI arguments in functional groups #442
Group CLI arguments in functional groups #442
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the way you grouped the options, it greatly improves the readability of --help
! Thanks :)
It looks like the tests need to be adapted. Let us know if you need any pointers with this.
Two minor, somewhat subjective, points:
- Should we move
--non-interactive
into the execution group? It's commonly used to select a different code path when in CI. By itself, it does not change Nox's operation or reporting, it's up to the session function to decide what to do with it. - Should we shorten "arguments & flags" to just "arguments" (or "options")?
Thanks for the feedback @cjolowicz!
This should fixed by 8627404, I had non-committed changes when I ran the tests locally 😓
Makes sense, I'm not yet fully familiar with the project so I thought had more to do with verbosity. I will make the change later today.
Just to confirm, I would change it only in the group title, not in the group description? e.g.:
|
This looks great, I'm happy with it once @cjolowicz is. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tests are passing now.
My suggestions are really optional, and just the group titles are enough. If you find time to do this later, great, otherwise I'm happy for this to be merged as-is.
Replace the primary/secondary argparse argument groups with functional groups: - general - sessions - python - environment - execution - reporting
cadda3e
to
1bd1153
Compare
@cjolowicz, I moved You can merge if you are satisfied with the changes. Thx. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #305
Replace the primary/secondary argparse argument groups with functional groups:
This is a first pass, I'm open for comments & suggestions on the arguments grouping, group naming and descriptions.