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

Now that getting the IP is much more reliable, using the forwarded port ...#189

Merged
SvenDowideit merged 1 commit into
boot2docker:masterfrom
SvenDowideit:fix-non-forwarded-docker-port
Jul 10, 2014
Merged

Now that getting the IP is much more reliable, using the forwarded port ...#189
SvenDowideit merged 1 commit into
boot2docker:masterfrom
SvenDowideit:fix-non-forwarded-docker-port

Conversation

@SvenDowideit
Copy link
Copy Markdown
Contributor

...here is much more dangerous

closes #118

@SvenDowideit
Copy link
Copy Markdown
Contributor Author

@tianon @gmlewis @crosbymichael @steeve I'm putting this into the 1.1.1 I'm about to release

@tianon
Copy link
Copy Markdown
Contributor

tianon commented Jul 10, 2014

Much constant, such magic number, many duplication, so undry.

Comment thread cmds.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about putting this near the beginning of this file and use it in the 3 places this file uses 2375?

const dockerPort = 2375

@gmlewis
Copy link
Copy Markdown
Contributor

gmlewis commented Jul 10, 2014

otherwise, LGTM

@SvenDowideit
Copy link
Copy Markdown
Contributor Author

only because it'd be 2 more lines of change :) one 🕛

@SvenDowideit
Copy link
Copy Markdown
Contributor Author

@tianon ! - though its not a shared constant - its only local to that file, which made me cry.

SvenDowideit pushed a commit that referenced this pull request Jul 10, 2014
Now that getting the IP is much more reliable, using the forwarded port ...
@SvenDowideit SvenDowideit merged commit 496ea22 into boot2docker:master Jul 10, 2014
@SvenDowideit
Copy link
Copy Markdown
Contributor Author

making assumptions - and kicking off an automated build

@SvenDowideit SvenDowideit deleted the fix-non-forwarded-docker-port branch July 10, 2014 04:29
@tianon
Copy link
Copy Markdown
Contributor

tianon commented Jul 10, 2014

Much better, and LGTM now.

SvenDowideit added a commit to SvenDowideit/boot2docker-cli that referenced this pull request Nov 10, 2014
Documentation for the workarounds
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