Skip to content

Conversation

larkost
Copy link
Contributor

@larkost larkost commented Jan 30, 2018

This adds a ports property to the Container object to list currently forwarded ports.

This should be the answer to #1742

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "1743-container-ports" git@github.com:larkost/docker-py.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@larkost
Copy link
Contributor Author

larkost commented Feb 18, 2018

Just in case... this was signed 19 days ago.

@larkost
Copy link
Contributor Author

larkost commented Feb 28, 2018

@TomasTomecek Could you please respond to my responses? This PR is getting a bit stale.

@TomasTomecek
Copy link
Contributor

I'm not a maintainer of this project, my comments are pretty much just my opinions. For a proper code review you need to wait for @shin-

@backtrackbaba
Copy link

Hey @shin- any update on this PR?

@yxliang01
Copy link

@larkost @ulyssessouza Any progress on this PR? It has been a year and this is super sensible and useful. Hope this can be merged.

larkost and others added 2 commits March 26, 2019 16:55
Signed-off-by: Karl Kuehn <kuehn.karl@gmail.com>
Signed-off-by: Ulysses Souza <ulysses.souza@docker.com>
@ulyssessouza ulyssessouza force-pushed the 1743-container-ports branch from 546c82a to d1f7979 Compare March 26, 2019 16:30
@ulyssessouza ulyssessouza dismissed their stale review March 26, 2019 16:50

Cuz I have a commit on it now :D

Copy link
Member

@chris-crone chris-crone left a comment

Choose a reason for hiding this comment

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

LGTM

@ulyssessouza ulyssessouza merged commit 1b572a4 into docker:master Apr 3, 2019
@yxliang01
Copy link

yxliang01 commented Apr 3, 2019

Great! Happy to see this is finally merged! Thanks guys!

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.

7 participants