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

Resolves #5: adds port selection via command line arguments #11

Merged
merged 1 commit into from Oct 28, 2019

Conversation

@Silvyre
Copy link
Contributor

Silvyre commented Oct 27, 2019

Resolves #5:

  • main.go now accepts command line arguments (via os.Args), replacing hardcoded port numbers.
  • Dockerfile now accepts build-time variables and sets additional environment variables, which replace hardcoded port numbers.
  • Executing make (i.e. make run) passes the values of the pre-existing EXPOSED_API_PORT and PROXY_PORT variables to main.go and to Dockerfile.

Examples of use:

$ make EXPOSED_API_PORT=9996 PROXY_PORT=9997
$ make docker-image EXPOSED_API_PORT=9996 PROXY_PORT=9997
$ make docker-run EXPOSED_API_PORT=9996 PROXY_PORT=9997
$ make docker-image EXPOSED_API_PORT=9995
$ make docker-run
@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Oct 27, 2019

CLA assistant check
All committers have signed the CLA.

@alextarasov1 alextarasov1 self-requested a review Oct 28, 2019
@alextarasov1

This comment has been minimized.

Copy link
Member

alextarasov1 commented Oct 28, 2019

Good work! For the executable I would have preferred to use command line flags, https://gobyexample.com/command-line-flags. This gives your command line arguments names and labels, and also makes providing default values easy. With default values the program won't crash if no arguments are provided, it also does type casting for you.

@alextarasov1 alextarasov1 merged commit 9ff695a into Comcast:master Oct 28, 2019
1 check passed
1 check passed
license/cla Contributor License Agreement is signed.
Details
@Silvyre Silvyre deleted the Silvyre:issue-5-ports-via-command-line branch Oct 28, 2019
@Silvyre

This comment has been minimized.

Copy link
Contributor Author

Silvyre commented Oct 28, 2019

Thank you very much! I'll definitely look into command line flags for my future Go work!

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