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

Deprecate Stats#getCountAsString #24292

Merged
merged 1 commit into from
Apr 24, 2017

Conversation

cbuescher
Copy link
Member

We don't render an "count_as_string" in the rest response,
so the method should also be removed in favour of just using
String.valueOf(getCount()) if a string version of the count is needed.

We don't render an "count_as_string" in the rest response,
so the method should also be removed in favour of just using
String.valueOf(getCount()) if a string version of the count is needed.
@javanna
Copy link
Member

javanna commented Apr 24, 2017

Why do we need to deprecate this? I think with java api changes we ask users to rely on their compilers. We made such changes already without deprecation in 5.x.

@colings86
Copy link
Contributor

I think its still ok for us to deprecate things where we have the chance to. The removal of this is happening in 6.0 so I think its good to deprecate it in 5.x even if its not required

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cbuescher
Copy link
Member Author

@colings86 thanks, will merge this to 5.x. I also agree that its better to add a deprecation note if it is possible, especially since this is a user facing interface.

@cbuescher cbuescher merged commit fb35812 into elastic:5.4 Apr 24, 2017
@javanna
Copy link
Member

javanna commented Apr 24, 2017

Not a huge deal but I don't see why we have to go case by case. As much as I may not like it, the policy is that we deprecate things on the REST layer, and java API users rely on their compiler. If we use deprecation for this we should do it across the board which we don't. It makes little sense to do it here then.

cbuescher added a commit that referenced this pull request Apr 24, 2017
We don't render an "count_as_string" in the rest response,
so the method should also be removed in favour of just using
String.valueOf(getCount()) if a string version of the count is needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants