Update bash-completion #2857
Update bash-completion #2857
Conversation
deployment/pypi/setup.py
Outdated
| @@ -15,6 +17,34 @@ | |||
| else: | |||
| raise NotImplementedError('current platform {} not supported'.format(os_type)) | |||
|
|
|||
| class AutoCompletion(install): | |||
| def run(self): | |||
| COMP_URL = 'https://raw.githubusercontent.com/microsoft/nni/master/tools/bash-completion' | |||
There was a problem hiding this comment.
it is not good to use master branch here
There was a problem hiding this comment.
why not pack it into python package?
There was a problem hiding this comment.
Sure, will change to a specific release branch.
If we pack it into python package, it is hard to locate the root of the python package, so I choose to directly download it.
| @@ -1,38 +1,42 @@ | |||
| # list of commands/arguments | |||
There was a problem hiding this comment.
is it possible to automatically generate this file? @liuzhe-lz
There was a problem hiding this comment.
Maybe we can just write the whole file's content to the correct location.
deployment/pypi/setup.py
Outdated
| @@ -48,7 +79,7 @@ | |||
| 'nnicli': '../../src/sdk/pycli/nnicli' | |||
| }, | |||
| package_data = {'nni': ['**/requirements.txt']}, | |||
| python_requires = '>=3.5', | |||
| python_requires = '>=3.6', | |||
There was a problem hiding this comment.
v1.8 no longer support python 3.5, see #2778
deployment/pypi/setup.py
Outdated
| if os_type == 'Linux': | ||
| HOME = os.environ.get('HOME') | ||
| if not HOME: | ||
| install.run(self) |
There was a problem hiding this comment.
what is install.run(self) used for here?
There was a problem hiding this comment.
Not quiet sure, seems that this line actually do the install action. Every example about how to use cmdclass has this line, like this link
deployment/pypi/setup.py
Outdated
| @@ -15,6 +17,35 @@ | |||
| else: | |||
| raise NotImplementedError('current platform {} not supported'.format(os_type)) | |||
|
|
|||
| class AutoCompletion(install): | |||
There was a problem hiding this comment.
better to remove installation, and update doc to explain how to enable bash completion, or output message after nni installation to tell users to enable bash completion
There was a problem hiding this comment.
Installation removed. An tutorial documentation has been added.
|
@QuanluZhang @SparkSnail @scarlett2018 @liuzhe-lz Since this feature will be refactored in v2.0, now I provide a tutorial to guide users how to install bash-completion instead of installing it with |
| @@ -0,0 +1,39 @@ | |||
| # Auto Completion | |||
There was a problem hiding this comment.
"Auto Completion" -> "Auto Completion of nnictl Commands"
Update tools/bash-completion, adding new commands and options, resolve previous bugs.
Also update some lines in docs/en_US/Nnictl.md and python version requirements.