-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
removal of unnecssary parsing if httpMethod is null #2218
removal of unnecssary parsing if httpMethod is null #2218
Conversation
@@ -282,6 +282,8 @@ private void parseHttpMethodAndPath(String httpMethod, String value, boolean has | |||
throw methodError("Only one HTTP method is allowed. Found: %s and %s.", | |||
this.httpMethod, httpMethod); | |||
} | |||
if (null == httpMethod) return; |
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.
This will never be true.
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 we go through the code in the ServiceMethod build after parseMethodAnnotations , we are checking if httpMethod is null or not and then throwing the exception , so if we check at parseHttpMethodPath itself , we don't need to do value parsing for updating relativeUrl
The exception checks that the method isn't called twice. The value of
httpMethod is guaranteed to never be null by Java since it comes from an
annotation.
…On Sat, Feb 25, 2017, 12:23 AM guptasourabh ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In retrofit/src/main/java/retrofit2/ServiceMethod.java
<#2218 (comment)>:
> @@ -282,6 +282,8 @@ private void parseHttpMethodAndPath(String httpMethod, String value, boolean has
throw methodError("Only one HTTP method is allowed. Found: %s and %s.",
this.httpMethod, httpMethod);
}
+ if (null == httpMethod) return;
if we go through the code in the ServiceMethod build after
parseMethodAnnotations , we are checking if httpMethod is null or not and
then throwing the exception , so if we check at parseHttpMethodPath itself
, we don't need to do value parsing.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2218 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEEEb9dVqwIFvPesDIZHU4WQc4sZ3NQks5rf7rUgaJpZM4MLkxw>
.
|
@JakeWharton requested changes done |
Moved it into new PR #2219 |
If httpMethod is null then we don't need to parse and set the other values