-
Notifications
You must be signed in to change notification settings - Fork 132
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
Extract code from cli.main into shorter methods #68
Conversation
I'd love to see this go in, the current main function is unnecessarily hard to get to grips with, which is holding up progress on other issues. |
It's definitely an improvement, although I've found that doing heavy modification of the command-line arguments is still... quite painful :-) Maintainers, can this be considered for merging? Anything that needs to be done differently? |
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.
Overall it looks like a nice improvement.
Pylint config: see c4urself#67
Also avoid reading the file twice.
(arguments which are modified in-place instead of copied/modified/returned)
These used to come along with installing tox, but this is no longer the case since version 3.10. See similar fix h5py/h5py@b183c74 and tox issue tox-dev/tox#1304 .
This is now rebased on master + I have applied all the suggestions. |
This got lost in the recent rebase.
Similar to its class VersionConfig.
Versus context and vcs_context.
Handle the None case in VersionConfig.parse.
With this update, the branch conforms to all remarks. |
This does not change any functionality, but it makes main() much shorter and at least
helps to decode what's going on.
Done as a preparation for implementing #57, which has impact on argument parsing.
I've made sure that
cli.py
satisfiespylint
similar to what's done in #67.Merging this with
#67
would give merge conflicts, but this way it does not require manual resolution.