Skip to content
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

fixes 1554116 - show resource-get in help-tool #6200

Merged
merged 1 commit into from Sep 9, 2016
Merged

fixes 1554116 - show resource-get in help-tool #6200

merged 1 commit into from Sep 9, 2016

Conversation

natefinch
Copy link
Contributor

@natefinch natefinch commented Sep 9, 2016

There were two problems here. The first is that we weren't registering the resource-get command when running the client... buit we need to, so we can run help-tool.

This then revealed a second problem - help-tool was passing in a dummy hook context into the command creation function, and hoped it wasn't actually looking too closely at the value. Well, the resource-get command creation function was looking fairly closely at that value. Luckily, it doesn't really need to, so I delayed that until we actually run the command.

A better general solution (proposed by Andrew) would be to pass the hook context into Run itself, since it's really only information that should need to be looked at when the command is actually run. But that's a bigger change.

http://reviews.vapour.ws/r/5636/

@natefinch
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Sep 9, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit c5326a9 into juju:master Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants