Skip to content

Conversation

@shykes
Copy link
Contributor

@shykes shykes commented Aug 9, 2014

  • Move "pull" to graph/pull.go
  • Move "push" to graph/push.go
  • Move "build" to daemon/build.go
  • Remove the deprecated server package
  • Simplify daemon creation: call NewDaemon() straight from main()

@shykes
Copy link
Contributor Author

shykes commented Aug 9, 2014

Ping @crosbymichael @tibor @LKD4D @unclejack

Note: legacy make test-integration seems to be broken, but I can't find the root cause at the moment. Presumably I upset an arcane hidden dependency somewhere... Any help would be appreciated.

Unit tests and CLI integration tests seem to pass well.

@LK4D4
Copy link
Contributor

LK4D4 commented Aug 9, 2014

Pretty much fails :) I'll take a look.

@tiborvass
Copy link
Contributor

I indeed, don't see why the MTU and network init configs should not be done inside NewDaemon.

@shykes
Copy link
Contributor Author

shykes commented Aug 9, 2014

Thanks @tiborvass, that was it.

For now I'm leaving a FIXME to cleanup how daemonconfig is managed. Now that there isn't
a initserver job to convert an env from, I don't think we even need a daemonconfig package
at all... But let's worry about that in a separate patch.

Ready for review.

@shykes
Copy link
Contributor Author

shykes commented Aug 9, 2014

This depends no #7496.

@tiborvass
Copy link
Contributor

LGTM, but don't merge before #7496

@shykes
Copy link
Contributor Author

shykes commented Aug 11, 2014

Ping

@LK4D4
Copy link
Contributor

LK4D4 commented Aug 11, 2014

LGTM

@tiborvass
Copy link
Contributor

This has a commit (with a different commitid) from the PR #7496 that has already been merged. Not sure if it's a problem.

@LK4D4
Copy link
Contributor

LK4D4 commented Aug 11, 2014

@tiborvass it should be removed I think

@crosbymichael
Copy link
Contributor

@shykes do you have time to rebase this PR?

@shykes
Copy link
Contributor Author

shykes commented Aug 11, 2014

Yes will do this afternoon

On Monday, August 11, 2014, Michael Crosby notifications@github.com wrote:

@shykes https://github.com/shykes do you have time to rebase this PR?


Reply to this email directly or view it on GitHub
#7497 (comment).

Signed-off-by: Solomon Hykes <solomon@docker.com>
@shykes
Copy link
Contributor Author

shykes commented Aug 13, 2014

Rebased

@LK4D4
Copy link
Contributor

LK4D4 commented Aug 13, 2014

@shykes gofmt failed

Victory!

Signed-off-by: Solomon Hykes <solomon@docker.com>
@shykes
Copy link
Contributor Author

shykes commented Aug 13, 2014

#@^%#*@&^@^$@ fixed

@LK4D4
Copy link
Contributor

LK4D4 commented Aug 13, 2014

LGTM

@shykes
Copy link
Contributor Author

shykes commented Aug 13, 2014

@crosbymichael
Copy link
Contributor

LGTM

crosbymichael pushed a commit that referenced this pull request Aug 13, 2014
Remove deprecated server package
@crosbymichael crosbymichael merged commit c9e359c into moby:master Aug 13, 2014
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.

4 participants