-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Feature/replace deprecated body #959
Feature/replace deprecated body #959
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.
Overall change is good.
Just unsure if should remove deprecated method
* @see #charset() | ||
* @deprecated use {@link #requestBody()} instead | ||
*/ | ||
public byte[] body() { |
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 don't recall when we deprecated this.
If was on 9.x think is ok to drop it.
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.
If deprecated method is completely removed from usage, is not that ok to remove the method itself? I think removing it prevents further usage. Should I keep it or remove it?
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.
A decoder
may exists out there that still uses it.
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 see.. Sorry I am not fully familiar on source code. I'll put it back.
Just a question out of this context, is it acceptable to use a 3rd party library (like apache commons lang) for some requirement?
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.
core has being clean from external dependencies and we are most likely to want to keep it that way.
Particularly about apache commons-*, every time I see them I read "old deprecated code that somehow still survived" =)
And one more small housekeeping note, can you please update the description of this PR with the following format:
This helps us keep track of what issue the PR is for and provides us with background on your thinking and the intended outcome of this change. |
I updated description. |
Thanks for your contribution! |
In this pr the old `body()` method calls replaced with `requestBody().asBytes()` method which both exists in Request class. The intention is to remove deprecated code and keep source code clean. Related to OpenFeign#857 * replaced old body with new Body.asBytes()
In this pr the old `body()` method calls replaced with `requestBody().asBytes()` method which both exists in Request class. The intention is to remove deprecated code and keep source code clean. Related to #857 * replaced old body with new Body.asBytes()
In this pr the old `body()` method calls replaced with `requestBody().asBytes()` method which both exists in Request class. The intention is to remove deprecated code and keep source code clean. Related to #857 * replaced old body with new Body.asBytes()
In this pr the old
body()
method calls replaced withrequestBody().asBytes()
method which both exists inRequest
class. The intention is to remove deprecated code and keep source code clean.