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

Nomenclature fix #441

Closed
johnnybubonic opened this issue Aug 2, 2021 · 3 comments · Fixed by #480
Closed

Nomenclature fix #441

johnnybubonic opened this issue Aug 2, 2021 · 3 comments · Fixed by #480
Assignees

Comments

@johnnybubonic
Copy link

Currently, SetHostURL() is a bit ambiguous in its wording. A more appropriate term would perhaps instead be "SetBaseURL" or the like. "HostURL" sounds more like specifying a Host header to me. Alternatively, this can be made a bit more clear by specifying a path other than / in the example in the documentation, such as client.SetHostURL("http://httpbin.org/some/path") ("Unique settings at Client level").

Take it or leave it, but would have saved me five minutes. 👍

@jeevatkm jeevatkm modified the milestone: v2.7.0 Milestone Sep 12, 2021
@jeevatkm
Copy link
Member

@johnnybubonic Thanks for reaching out and sharing your thoughts.
Yeah, the name SetBaseURL suitable name too. This is a breaking change, so parking it for v3.0.0

@jeevatkm jeevatkm added this to the v3.0.0 Milestone milestone Sep 12, 2021
@jeevatkm jeevatkm added the v3 For resty v3 label Sep 12, 2021
@jeevatkm
Copy link
Member

@johnnybubonic Had another thought. Possibly we can add a new method SetBaseURL and deprecated the SetHostURL. So that on v3.0.0, it will become feasible to remove deprecated elements.

@johnnybubonic
Copy link
Author

That would be more sensible an approach, i think

@jeevatkm jeevatkm added enhancement and removed v3 For resty v3 labels Oct 24, 2021
@jeevatkm jeevatkm self-assigned this Oct 25, 2021
@jeevatkm jeevatkm mentioned this issue Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants