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

Support parsing a single port spec #14

Merged
merged 1 commit into from
Jun 8, 2016

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented May 27, 2016

I extracted the port spec parsing into a function that supports an individual spec.

@dnephin dnephin force-pushed the support_parsing_a_single_port_spec branch from 1d56d2d to ed26d18 Compare May 30, 2016 23:57
@dnephin
Copy link
Contributor Author

dnephin commented May 31, 2016

Fixed the vet failures

@calavera
Copy link
Contributor

can we have some tests for that function? otherwise LGTM

@dnephin dnephin force-pushed the support_parsing_a_single_port_spec branch from ed26d18 to 0be71da Compare June 1, 2016 23:54
@dnephin
Copy link
Contributor Author

dnephin commented Jun 1, 2016

This code is already pretty well covered by the old tests, but I added another one to test it directly.

@calavera
Copy link
Contributor

calavera commented Jun 2, 2016

awesome! 🤘 LGTM

@thaJeztah
Copy link
Member

looks like nat_test.go needs a gofmt

test -z "$(gofmt -s -l . | tee /dev/stderr)"
nat/nat_test.go

test -z "$(gofmt -s -l . | tee /dev/stderr)" returned exit code 1

In addition to parsing a slice of port specs.

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
@dnephin dnephin force-pushed the support_parsing_a_single_port_spec branch from 0be71da to fa2850f Compare June 4, 2016 17:08
@dnephin
Copy link
Contributor Author

dnephin commented Jun 4, 2016

Fixed the gofmt

@icecrime
Copy link

icecrime commented Jun 8, 2016

LGTM

@icecrime icecrime merged commit 990a1a1 into docker:master Jun 8, 2016
@dnephin dnephin deleted the support_parsing_a_single_port_spec branch June 8, 2016 02:45
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.

None yet

4 participants