Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse port ranges #544

Merged
merged 5 commits into from
Apr 22, 2015
Merged

parse port ranges #544

merged 5 commits into from
Apr 22, 2015

Conversation

yuval-k
Copy link
Contributor

@yuval-k yuval-k commented Apr 11, 2015

Added parsing port range utility.
to assist with:
docker/compose#1102
see:
docker/compose#1191 (comment)
for more details.

yuval-k added 4 commits April 3, 2015 17:23
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
to_port_range,
split_port,
add_port_mapping,
build_port_bindings
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think to_port_range and add_port_mapping should be imported here - they're only used within ports.py, and I can't see them being useful outside it

@aanand
Copy link
Contributor

aanand commented Apr 17, 2015

Looks good, apart from minor API surface area issue (comment inline).

Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@yuval-k
Copy link
Contributor Author

yuval-k commented Apr 17, 2015

@aanand - thanks for the feedback - fixed

@shin-
Copy link
Contributor

shin- commented Apr 22, 2015

LGTM, thanks for taking the time to do this!

shin- added a commit that referenced this pull request Apr 22, 2015
@shin- shin- merged commit 4267b9a into docker:master Apr 22, 2015
@shin- shin- modified the milestone: 1.2.0 Apr 22, 2015
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.

3 participants