{"action":"created","issue":{"url":"https://api.github.com/repos/docker/machine/issues/2187","labels_url":"https://api.github.com/repos/docker/machine/issues/2187/labels{/name}","comments_url":"https://api.github.com/repos/docker/machine/issues/2187/comments","events_url":"https://api.github.com/repos/docker/machine/issues/2187/events","html_url":"https://github.com/docker/machine/pull/2187","id":115328837,"number":2187,"title":"[WIP] Inject the Store into commands","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},"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":3,"created_at":"2015-11-05T17:03:28Z","updated_at":"2015-11-06T02:05:24Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/docker/machine/pulls/2187","html_url":"https://github.com/docker/machine/pull/2187","diff_url":"https://github.com/docker/machine/pull/2187.diff","patch_url":"https://github.com/docker/machine/pull/2187.patch"},"body":"Testing the commands is still complicated. So I carried on with refactoring the code:\r\n\r\n+ Inject the Store into command -> easy to mock.\r\n+ Cleanup createInner and createOuter -> it's a mess to read and test.\r\n+ Replace most helper methods on store be calls to store directly -> Easier to mock\r\n+ Improve error messages by removing some long error chains that say the same thing 3x\r\n\r\nSigned-off-by: David Gageot <david@gageot.net>"},"comment":{"url":"https://api.github.com/repos/docker/machine/issues/comments/154262092","html_url":"https://github.com/docker/machine/pull/2187#issuecomment-154262092","issue_url":"https://api.github.com/repos/docker/machine/issues/2187","id":154262092,"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},"created_at":"2015-11-06T02:05:24Z","updated_at":"2015-11-06T02:05:24Z","body":"Let's change just one piece at a time (i.e. let's not go modifying the `cmdOuter` and `cmdInner` stuff at the same time as working on injecting a libmachine client).\r\n\r\nThe `cmdOuter` and `cmdInner` stuff, while I know it _looks_ ugly and horrible, is that way for very specific reasons.  There has been a lot of effort put into making codegangsta/cli work with the types of dynamic lookups we're doing with pluggable drivers and I doubt we can take out large chunks safely overnight.  \r\n\r\nIt's split up into two functions for a very specific reason, for instance:  If `SkipFlagParsing` is not set to `true` for the outermost `create` function, codegangsta/cli will complain about flags it doesn't recognize (because it can't have flags provided by drivers compiled directly in, it doesn't know them ahead of time).  But the flag parsing actually _does_ need to be done for the second time around, in order to validate flags passed in and to parse them out into a form queryable by e.g. `c.Bool(\"blah\")`.\r\n\r\nGranted, I'm not averse to cleaning it up if possible, but we need to be really careful about not breaking the fragile logic around `create` in its current form.  And it should most likely be done in a few separate PRs.  This one purports to only deal with injecting store / `libmachine` client into the commands."}}