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

Questions about EurekaConnection.generateURL() #9

Open
cquinn opened this issue Jun 25, 2014 · 2 comments
Open

Questions about EurekaConnection.generateURL() #9

cquinn opened this issue Jun 25, 2014 · 2 comments

Comments

@cquinn
Copy link
Contributor

cquinn commented Jun 25, 2014

It seems like generateURL() could be use to completely assemble the URL from its slug parts just by passing the parts as varargs. But none of the calling code actually uses this feature, and instead assembles most of the URL with a couple different approaches.

E.g. in RegisterInstance(), two approaches in combination are used, Sprintf and later string cat:

slug := fmt.Sprintf("%s/%s", EurekaURLSlugs["Apps"], ins.App)
reqURL := e.generateURL(slug)
_, rcode, err := getBody(reqURL+"/"+ins.HostName, e.UseJson)

And it seems that using generateURL() to do the work would be simpler:

reqURL := e.generateURL(EurekaURLSlugs["Apps"], ins.App, ins.HostName)

Second question: what purpose does the EurekaURLSlugs[] lookup serve if it only ever maps a string to a nearly identical string. Maybe we can just remove it? Then the above would be:

reqURL := e.generateURL("apps", ins.App, ins.HostName)

I can test this and prepare a PR if it makes sense to you.

@tysonstewart
Copy link
Contributor

These changes make sense to me. I'm not sure if @ryansb has any objections, though.

@ryansb
Copy link
Contributor

ryansb commented Jul 28, 2014

Oops, I read and (thought) I'd 👍'd this already.

In any case, sounds great.

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

No branches or pull requests

3 participants