-
Notifications
You must be signed in to change notification settings - Fork 281
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
Remove scope method from the provider interface #402
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh. Did Scope() used to do something else? Or was it always equivalent to Get()?
CHANGELOG.md
Outdated
@@ -1,6 +1,7 @@ | |||
# Changelog | |||
|
|||
## v1.0.0-beta3 (unreleased) | |||
* Remove `Scope` method from the `config.Provider` interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mention why briefly? "because it was unused" or whatever
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this is potentially [BREAKING]
right, since people could be calling Scope() in their code?
Scope supposed to return a provider that appends a prefix to the current provider. We implemented it for every provider differently, but I've changed to use the scoped provider underneath everywhere a couple month ago, and with addition of Get method to Value, we can simply remove the repeated code and method from the interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!! Scope always bothered me here, Get is much nicer.
87d2352
to
4e92ff9
Compare
@peter-edge mentioned, that using Scope in Provider is very clumsy and we actually don't really need every Provider to implement Scope method, Get should be enough. Users can either use ScopedProvider to create a new scope for a provider or we can add this functionality to the Value. Users can simply call:
config.Get("yarpc").Get("http").Get("port")
instead of
config.Scope("yarpc").Scope("http").Get("port")