Skip to content

Adds flag for changing the default domain.dev #71

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

Merged
merged 6 commits into from
Oct 24, 2017

Conversation

cstadach
Copy link
Contributor

@cstadach cstadach commented Oct 24, 2017

these changes should make it possible to change the default .dev domain to whatever you want
resolves #70

@cstadach cstadach changed the title Adds flag for changing the default domain.dev #70 Adds flag for changing the default domain.dev Oct 24, 2017
@codecov-io
Copy link

codecov-io commented Oct 24, 2017

Codecov Report

Merging #71 into master will decrease coverage by 0.44%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #71      +/-   ##
==========================================
- Coverage   82.41%   81.97%   -0.45%     
==========================================
  Files          10       10              
  Lines         290      294       +4     
==========================================
+ Hits          239      241       +2     
- Misses         37       38       +1     
- Partials       14       15       +1
Impacted Files Coverage Δ
commands/run.go 0% <0%> (ø) ⬆️
main.go 70.58% <50%> (-1.29%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00c29dd...9aa188e. Read the comment docs.

Copy link
Owner

@cristianoliveira cristianoliveira left a comment

Choose a reason for hiding this comment

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

Hey @cstadach thanks for your contributions.

I left some comments about your PR. Please take a look on them.

@@ -98,6 +99,9 @@ func command() func() {
command.BoolVar(&config.Verbose, "V", false, "Set verbosity on proxy output")

command.Parse(os.Args[2:])
if !strings.HasPrefix(config.Domain, ".") {
Copy link
Owner

Choose a reason for hiding this comment

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

Could we add a test for it? We already have some tests that cover returning no command it can be used as example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the test in commit 9aa188e

commands/run.go Outdated
@@ -13,6 +14,7 @@ import (
func Run(config *proxy.Config) {

fmt.Println("Ergo Proxy listening on port: " + config.Port)
fmt.Println("Ergo Proxy listening for domain: " + config.Domain)
Copy link
Owner

Choose a reason for hiding this comment

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

Could we avoid repeat the "Ergo proxy listening" sentence? How about?

fmt.Println("Ergo Proxy listening on port " + config.Port + " for domains " + config.Domain)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea.
I pushed the text changes already.

main.go Outdated
@@ -30,6 +31,7 @@ Options:
-h Shows this message.
-v Shows ergo's version.
-config Set the config file to the proxy.
-domain Set the domain the proxy is running.
Copy link
Owner

Choose a reason for hiding this comment

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

This sounds strange. How about "Set a custom domain for services"?

main.go Outdated
@@ -49,6 +51,7 @@ func command() func() {
config := proxy.NewConfig()
command := flag.NewFlagSet(os.Args[1], flag.ExitOnError)
configFile := command.String("config", "./.ergo", "Set the services file")
domain := command.String("domain", ".dev", "Set the doamin for the proxy service")
Copy link
Owner

Choose a reason for hiding this comment

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

Same suggestion as above

@cristianoliveira
Copy link
Owner

Hey @cstadach Thanks!

@cristianoliveira cristianoliveira merged commit b73c031 into cristianoliveira:master Oct 24, 2017
@ghost
Copy link

ghost commented Dec 14, 2020

usage for -domain flag added in #71:

ergo run -domain .test
ERGO_DOMAIN=.test ergo run

@thinsoldier
Copy link

Is it possible to use with more than one domain extension?

I was using some vagrant boxes that had their own IP and no need for ports.

So my urls were like:

project4.oldemployer
project12.newemployer
project23.sidehustle
project8.family
project3.friend

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.

feature: Add flag for changing the default domain.dev
4 participants