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

Accept a comma-separated list of ports to forward #16

Merged
merged 4 commits into from Aug 3, 2019

Conversation

@jkutner
Copy link
Contributor

commented Jul 18, 2019

Supersedes #15

jclem and others added 2 commits Jul 11, 2019

@jkutner jkutner requested review from beanieboi, RasPhilCo and heroku/cli Jul 18, 2019

@RasPhilCo
Copy link
Contributor

left a comment

🏄

@beanieboi

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

@jkutner Do we need backend changes for this? Otherwise looks good to me

@jclem

This comment has been minimized.

Copy link

commented Jul 19, 2019

Should we remove the localPort option and its documentation?

@jkutner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2019

@beanieboi nope, no backend changes. I added you just to get another set of eyes on it.

@jkutner

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2019

@jclem oh hmm, i didn't notice that localPort was affected. I need to think on that.

@jclem

This comment has been minimized.

Copy link

commented Jul 19, 2019

@jkutner Perhaps we could make them mutually exclusive for backwards compatibility and just error out if both formats are used (the local port option and port:port or a list of ports), and then deprecate the local port argument.

commands/port.js Show resolved Hide resolved

@jkutner jkutner merged commit 3455707 into master Aug 3, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jkutner jkutner deleted the jclem-master branch Aug 3, 2019

@jclem

This comment has been minimized.

Copy link

commented Aug 5, 2019

🎉 Thanks @jkutner! When-ish will this be available in the CLI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.