-
Notifications
You must be signed in to change notification settings - Fork 867
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
Rename Config get*Property() methods to get*() #3881
Rename Config get*Property() methods to get*() #3881
Conversation
} | ||
|
||
/** | ||
* Returns a list-valued configuration property or {@code defaultValue} if a property with name | ||
* {@code name} has not been configured. The format of the original value must be comma-separated, | ||
* e.g. {@code one,two,three}. | ||
*/ | ||
public List<String> getListProperty(String name, List<String> defaultValue) { | ||
public List<String> getList(String name, List<String> defaultValue) { |
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.
I used simpler getList
/getMap
names instead of getCommaSeparatedValues
/getCommaSeparatedMap
that SDK uses -- all other method names are consistent with the ConfigProperties
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.
Seems like a nice change we can make to the SDK too (since it's alpha)
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/config/ConfigTest.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
/** | ||
* Returns a list-valued configuration property or {@code defaultValue} if a property with name | ||
* {@code name} has not been configured. The format of the original value must be comma-separated, | ||
* e.g. {@code one,two,three}. | ||
*/ | ||
public List<String> getListProperty(String name, List<String> defaultValue) { | ||
public List<String> getList(String name, List<String> defaultValue) { |
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.
Seems like a nice change we can make to the SDK too (since it's alpha)
b7d91a2
to
d97a923
Compare
*/ | ||
@Deprecated |
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.
I had to leave all those old methods and make them deprecated -- muzzle wouldn't let me remove them in this PR, I'll have to do that in the next one.
... and rewrite the rest of the Spock test to JUnit.
See #3866 (comment)