{"action":"created","issue":{"url":"https://api.github.com/repos/docker/machine/issues/2295","labels_url":"https://api.github.com/repos/docker/machine/issues/2295/labels{/name}","comments_url":"https://api.github.com/repos/docker/machine/issues/2295/comments","events_url":"https://api.github.com/repos/docker/machine/issues/2295/events","html_url":"https://github.com/docker/machine/pull/2295","id":116891288,"number":2295,"title":"Move to using more testable interfaces","user":{"login":"nathanleclaire","id":1476820,"avatar_url":"https://avatars.githubusercontent.com/u/1476820?v=3","gravatar_id":"","url":"https://api.github.com/users/nathanleclaire","html_url":"https://github.com/nathanleclaire","followers_url":"https://api.github.com/users/nathanleclaire/followers","following_url":"https://api.github.com/users/nathanleclaire/following{/other_user}","gists_url":"https://api.github.com/users/nathanleclaire/gists{/gist_id}","starred_url":"https://api.github.com/users/nathanleclaire/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nathanleclaire/subscriptions","organizations_url":"https://api.github.com/users/nathanleclaire/orgs","repos_url":"https://api.github.com/users/nathanleclaire/repos","events_url":"https://api.github.com/users/nathanleclaire/events{/privacy}","received_events_url":"https://api.github.com/users/nathanleclaire/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/docker/machine/labels/status/0-triage","name":"status/0-triage","color":"fef2c0"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":6,"created_at":"2015-11-14T03:10:58Z","updated_at":"2015-11-18T17:07:37Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/docker/machine/pulls/2295","html_url":"https://github.com/docker/machine/pull/2295","diff_url":"https://github.com/docker/machine/pull/2295.diff","patch_url":"https://github.com/docker/machine/pull/2295.patch"},"body":"It is a combination of @dgageot commits / refactoring work and my own.  Demonstration of increased testability and proper rebase coming soon.  This contains all the steps needed to get us to extremely testable code at the subcommand level."},"comment":{"url":"https://api.github.com/repos/docker/machine/issues/comments/157780388","html_url":"https://github.com/docker/machine/pull/2295#issuecomment-157780388","issue_url":"https://api.github.com/repos/docker/machine/issues/2295","id":157780388,"user":{"login":"dgageot","id":153495,"avatar_url":"https://avatars.githubusercontent.com/u/153495?v=3","gravatar_id":"","url":"https://api.github.com/users/dgageot","html_url":"https://github.com/dgageot","followers_url":"https://api.github.com/users/dgageot/followers","following_url":"https://api.github.com/users/dgageot/following{/other_user}","gists_url":"https://api.github.com/users/dgageot/gists{/gist_id}","starred_url":"https://api.github.com/users/dgageot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dgageot/subscriptions","organizations_url":"https://api.github.com/users/dgageot/orgs","repos_url":"https://api.github.com/users/dgageot/repos","events_url":"https://api.github.com/users/dgageot/events{/privacy}","received_events_url":"https://api.github.com/users/dgageot/received_events","type":"User","site_admin":false},"created_at":"2015-11-18T17:07:37Z","updated_at":"2015-11-18T17:07:37Z","body":"@nathanleclaire Thanks for the work, here's my feedback on a couple of points:\r\n\r\n+ Lot's commands use `c commandLine` to extract the args which are either a machine name or multiple machine names. This should be obvious and not hidden behind `GetFirstArgHost`, `GetHostsFromContext` and `RunActionWithContext` functions. What we really need is a `GetHost(machineName string)` and `RunAction(machineName []string)\r\n\r\n+ `MachineCLIHelper` is mixing things that are related to command line and the store.  Please give it a single responsibility.\r\n\r\n+ Do you think `CLIer` is a better name than `CommandLine`? In go, it's the norm to call an interface `XXer` when it has a single function called `XX`. This is not the case here. I'm confused.\r\n\r\n+ I don't think the `Filestore` should take care of loading plugin drivers. Let's group this code outside of `Filestore` to enforce a single responsibility to each struct and to make things easier to test.\r\n\r\n+ Also with this kind of refactoring, please consider splitting the thing into multiple commits to make it easier to review. For example, I heard you once ask for commits dedicated to moving functions around. I think it's a great idea that would have helped understand this PR."}}