-
Notifications
You must be signed in to change notification settings - Fork 11
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
node network list now calls from esisdk #65
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 can't fully test this, since there's an error I'm tracing back to esisdk
(which I'll comment on there). But broadly, I think it looks good; however you'll want to update the unit test.
21f0821
to
67d4aa0
Compare
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.
One minor comment; looks good to me otherwise!
@@ -24,7 +24,7 @@ class TestList(base.TestCommand): | |||
|
|||
def setUp(self): | |||
super(TestList, self).setUp() | |||
self.cmd = node_network.List(self.app, None) | |||
self.maxDiff = None |
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 don't think this is needed (at least from my testing).
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.
Ah that was for my debugging, I'll get rid of it now
Many projects want to use the functionality that previously existed in this project. That functionality has been moved to esisdk to centralize the logic, and to not repeat the same code.
Thanks! |
To centralize logic, the
esi node network list
will call from esisdk.