-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Improve nullability for gauges #6546
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
Improve nullability for gauges #6546
Conversation
3b2355a to
42023ac
Compare
| * @return The number that was passed in so the registration can be done as part of an | ||
| * assignment statement. | ||
| */ | ||
| @Contract("_, _, !null -> !null") |
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.
There are some inconsistencies including this. If the state object (number) being non-null is intentional, dropping @Nullable seems to be better.
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 just dropped @Nullable annotations for these cases instead.
| * assignment statement. | ||
| */ | ||
| @Contract("_, _, !null, _ -> !null") | ||
| public static <T> @Nullable T gauge(String name, Iterable<Tag> tags, T obj, ToDoubleFunction<T> valueFunction) { |
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.
There is another inconsistency: the state object in the corresponding method in the MeterRegistry is nullable while this state object (obj) is not nullable.
Signed-off-by: Johnny Lim <[email protected]>
42023ac to
b07ee6b
Compare
|
I think this is definitely an improvement over what we have now but we might need to revert parts of this:
/cc @shakuzen |
This PR changes to use
@Contractto improve nullability for gauges.