-
Notifications
You must be signed in to change notification settings - Fork 446
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
Refactor configuration & CLI #2805
Refactor configuration & CLI #2805
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've reviewed until cli.py. Will continue tomorrow
…nsions into harimkan/cli-configurations
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 @harimkang. I think I'm happy with the changes. Great effort!
…nsions into harimkan/cli-configurations
3214f22
Sorry for your inconvenience, but since the time zone is different, please resolve the conflict and merge after this PR #2764. |
Yes, of course. If that PR passes all the tests and then gets merged, I'll fix it here. |
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.
Since the integration tests of #2764 were failed and seem hard to fix this time, let merge this first.
Summary
I wanted to break up the PR as much as possible, but the CLI and configuration are still intertwined, so please excuse the large size of the PR. (Most files are YAML files.)
Next PR
How to test
Checklist
License
Feel free to contact the maintainers if that's a concern.