-
Notifications
You must be signed in to change notification settings - Fork 179
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
Adding GPU feature and some dependent tools #1318
Conversation
e4f07a3
to
ecb43c3
Compare
ecb43c3
to
5dbe723
Compare
5dbe723
to
81486b0
Compare
9b65372
to
e06d939
Compare
a3c407c
to
e1460ce
Compare
e1460ce
to
6bd0f36
Compare
6bd0f36
to
f45a5af
Compare
7293926
to
19b5e28
Compare
def update_packages( | ||
self, packages: Union[str, Tool, Type[Tool], List[Union[str, Tool, Type[Tool]]]] | ||
) -> None: | ||
raise NotImplementedError |
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.
Any use case for this? What's the role of a test suite in updating stuff?
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.
Looking good otherwise :)
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.
It might be used internally say - in case the some package/tool version queried doesn't match the required version then it would need to be updated first.
But yeah it most likely should not be taking in a list.
19b5e28
to
bab7c2a
Compare
Added separate install_packages_from_url() Moved wget class under base_tools/ Some cleanups and fixes.
bab7c2a
to
810e37b
Compare
No description provided.