Skip to content
This repository has been archived by the owner on Feb 27, 2018. It is now read-only.

Create driver model and refactor virtualbox dependencies into driver #184

Merged
merged 5 commits into from
Jul 18, 2014
Merged

Conversation

zeeyang
Copy link
Contributor

@zeeyang zeeyang commented Jul 8, 2014

Laying out some foundation work for multi-hypervisor support. Some keys changes include:

  • New Machine interface defined in driver/driver.go.
  • Move common data structures from virtualbox into driver package.
  • Eliminated all direct references to virtualbox package.
  • Added --driver flag. e.g. boot2docker-cli --driver=dummy up
  • Added --init flag for optional auto init. e.g. boot2docker-cli --init=true up

@SvenDowideit
Copy link
Contributor

I will look at it in more detail after I get the 1.1.1 b2d release out - very awesome to see though

@SvenDowideit
Copy link
Contributor

can we please not make init implicit? its useful to be able to separate out the creation from the starting

instead, perhaps we should add a --force which does whatever is needed.

@tianon
Copy link
Contributor

tianon commented Jul 9, 2014

I agree with @SvenDowideit that having an explicit init function (even if it's usually implicit) is useful, but all-in-all the refactor looks pretty solid from a visual code review - the interfaces look reasonably idiomatic and I'm excited to see where this goes.

@zeeyang
Copy link
Contributor Author

zeeyang commented Jul 9, 2014

Thanks for the feedback guys. I brought back the init command. Auto init is now explicit with the newly added --init=true (default false) flag.

@SvenDowideit
Copy link
Contributor

yes, I like it LGTM - @tianon @gmlewis @crosbymichael @steeve I'd love to merge this and then we can keep working on it :)

@tianon
Copy link
Contributor

tianon commented Jul 16, 2014

LGTM

@@ -294,8 +325,8 @@ func ListMachines() ([]string, error) {
}

// CreateMachine creates a new machine. If basefolder is empty, use default.
func CreateMachine(name, basefolder string) (*Machine, error) {
if name == "" {
func CreateMachine(i *driver.MachineConfig) (*Machine, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

tiny nit: I have been encouraged in peer reviews of my own code to reserve variables like i to be integer index counters for loops... so that others looking at the code are least surprised. So if you agree, you could make this variable mc or m. If you want to leave it as is, that's fine with me too.

@gmlewis
Copy link
Contributor

gmlewis commented Jul 16, 2014

LGTM after comments addressed.

@zeeyang
Copy link
Contributor Author

zeeyang commented Jul 16, 2014

Thanks for the code review. @SvenDowideit I have rebased to lastest master. There are three new commits based on @gmlewis feedback.

SvenDowideit pushed a commit that referenced this pull request Jul 18, 2014
Create driver model and refactor virtualbox dependencies into driver
@SvenDowideit SvenDowideit merged commit 711cc7d into boot2docker:master Jul 18, 2014
@SvenDowideit
Copy link
Contributor

awesome!

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.

4 participants