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.

Sign up for GitHub

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

Jump to bottom

Adds support for amp service create -p, and tests #197

Merged
merged 4 commits into from
Sep 20, 2016
Merged

Adds support for amp service create -p, and tests #197

merged 4 commits into from
Sep 20, 2016

Conversation

subfuzion
Copy link
Contributor

The CLI now supports amp service create -p where -p is the input string for a PublishSpec.

Verification

$ go test -run TestRegex github.com/appcelerator/amp/api/rpc/service -v
$ amp service create -p 3000:3000 appcelerator/pinger
$ curl localhost:3000/ping
pong

generalhenry
generalhenry suggested changes Sep 19, 2016
View reviewed changes
Copy link
Contributor

@generalhenry generalhenry left a comment

Choose a reason for hiding this comment

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

Pong working locally
Tests running into build issue.

api/rpc/service/helpers_test.go
{input: "80:", reason: "publish port must specify internal port"},
{input: "80:3000/", reason: "missing protocol"},
{input: "80:3000/x", reason: "invalid protocol"},
{input: "host-:4000:3000/udp", expect: PublishSpec{InternalPort: 3000, PublishPort: 4000, Protocol: "udp"}},
Copy link
Contributor

Choose a reason for hiding this comment

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

These lines are throwing a build error

freignat91
freignat91 reviewed Sep 19, 2016
View reviewed changes
Copy link
Contributor

@freignat91 freignat91 left a comment

Choose a reason for hiding this comment

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

I got same issues than Henry's

@subfuzion
Copy link
Contributor Author

Fixed the last two tests.

freignat91
freignat91 approved these changes Sep 20, 2016
View reviewed changes
Copy link
Contributor

@freignat91 freignat91 left a comment

Choose a reason for hiding this comment

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

tested ok

@subfuzion subfuzion merged commit 2615ea7 into master Sep 20, 2016
@bquenin bquenin deleted the service-port branch September 20, 2016 20:48
@subfuzion subfuzion added this to the 0.1.0 milestone Oct 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@generalhenry generalhenry generalhenry requested changes

@freignat91 freignat91 freignat91 approved these changes

Assignees
No one assigned
Labels
impact/api impact/changelog impact/cli
Projects
None yet
Milestone
0.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@subfuzion @generalhenry @freignat91