Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

wip: store re-org / internal api #550

Closed
wants to merge 2 commits into from

Conversation

ehazlett
Copy link
Contributor

This is a proposal / PR to make machine more maintainable. I want to get feedback on the direction. It does the following:

  • make github.com/docker/machine top level package
  • move cli to docker-machine
  • create an internal api that wraps store and machine operations (refs: WIP: Refactor code out of main package #286)
  • moves some non-store related operations (create/remove) to machine (this will enable more datastore providers in the future)
  • error types

@ehazlett ehazlett changed the title store re-org / internal api wip: store re-org / internal api Feb 18, 2015
@ehazlett
Copy link
Contributor Author

@@ -8,7 +8,7 @@ RUN go get github.com/tools/godep
RUN go get code.google.com/p/go.tools/cmd/cover

ENV GOPATH /go/src/github.com/docker/machine/Godeps/_workspace:/go
ENV MACHINE_BINARY /go/src/github.com/docker/machine/docker-machine
ENV MACHINE_BINARY /go/src/github.com/docker/machine/docker-machine/docker-machine
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just call this directory bin to avoid confusion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a problem with go get. go get names the binary by whatever directory the main package is in.

By putting it in a docker-machine dir, we can ensure that people will have a binary called docker-machine.

@nathanleclaire
Copy link
Contributor

I'll need a little time to look it over thoroughly, but having just run into some issues with circular imports I'm very in favor of the direction.

@sthulb
Copy link
Contributor

sthulb commented Feb 18, 2015

I'm still not fond of an api package :)

@ehazlett
Copy link
Contributor Author

@nathanleclaire there shouldn't be any import issues -- the tests pass and the binary builds. I will reset and see if I get the same.

@sthulb I know :) I'm up for debate - it seemed to fit. machine to me is the actual machine. We could do something like move it to the toplevel so it's just github.com/docker/machine -> NewMachine or something and perhaps move store into a subpackage - I was inclined to move that to a subpackage anyway since we will probably want pluggable stores in the future.

@nathanleclaire
Copy link
Contributor

@ehazlett Oh, I was commenting that I was running into circular imports with something (unrelated) that I was working on, and I think re-organizing might help with that.

@ehazlett
Copy link
Contributor Author

@nathanleclaire ah ok cool

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
…eate docker-machine package; update build script

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@ehazlett
Copy link
Contributor Author

ehazlett commented Mar 6, 2015

Closing as #741 implements most of the moving provisioning and create around and this has stagnated with not much discussion. It also looks like we will keep the Hosts name as part of #727

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants