Skip to content

Conversation

@creack
Copy link
Contributor

@creack creack commented Nov 22, 2013

Refactor *Opts in order to make the code more consistent and easy to read.
Before the PR, we had some opts that used Custom type, other the utils.ListOpts, some validations by type, some somewhere else.

This PR tries to clean things up.

@tianon
Copy link
Member

tianon commented Nov 25, 2013

This might be a good place to cherry-pick some of the new verbiage from shykes#63, to help cut down on merge work later.

@creack
Copy link
Contributor Author

creack commented Nov 25, 2013

@tianon: done

@tianon
Copy link
Member

tianon commented Nov 25, 2013

👍

@vieux
Copy link
Contributor

vieux commented Nov 25, 2013

@creack when you cherry picked, you imported:

flGraphDriver = flag.String("graph-driver", "", "Force docker runtime to use a specific graph driver")

Could you remove it (it's unused outside 0.7)

@creack
Copy link
Contributor Author

creack commented Nov 25, 2013

@vieux done

@vieux
Copy link
Contributor

vieux commented Nov 26, 2013

@creack this needs to be rebased

Guillaume J. Charmes added 2 commits November 26, 2013 17:46
@creack
Copy link
Contributor Author

creack commented Nov 26, 2013

@vieux done

@vieux
Copy link
Contributor

vieux commented Nov 26, 2013

@crosbymichael let's be careful on this one 😆

@creack
Copy link
Contributor Author

creack commented Nov 27, 2013

ping @vieux @crosbymichael @shykes. Unit tests are in #2892

@creack
Copy link
Contributor Author

creack commented Dec 2, 2013

ping @vieux @crosbymichael @shykes. Unit tests are in #2892

@vieux
Copy link
Contributor

vieux commented Dec 2, 2013

LGTM

1 similar comment
@crosbymichael
Copy link
Contributor

LGTM

crosbymichael added a commit that referenced this pull request Dec 2, 2013
@crosbymichael crosbymichael merged commit fe571dd into master Dec 2, 2013
@crosbymichael crosbymichael deleted the refactor_opts branch December 2, 2013 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants