-
Notifications
You must be signed in to change notification settings - Fork 949
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
Landing Okhttp3 #293
Landing Okhttp3 #293
Conversation
Migrating google-maps-services-java to OkHttp3.
requestHandler.setProxy(proxy == null ? Proxy.NO_PROXY : proxy); | ||
return this; | ||
/** | ||
* Disable retries completely. |
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.
you might note that this clears the time limit and retries.
My 2c: I find this method a bit hard to grok. Setting retries to zero should be just as effective (even with e.g., a ten hour timeout)—can we just get users to do that?
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.
JavaDoc made more explicit as to the actions of the method.
@@ -30,7 +30,7 @@ | |||
*/ | |||
public double speedLimit; | |||
|
|||
/** Returns the speed limit in miles per hour (MPH). */ | |||
/** @return Returns the speed limit in miles per hour (MPH). */ |
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.
Should you rephrase this now?
@return The speed limit in freedom units (MPH).
I mean, just drop the 'Returns'.
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've always written JavaDoc with Returns as the first word of the @return line, because the generated JavaDoc reads a tad weird without it. Or at least it did back in the 1.4 days...
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.
LGTM but maybe we should chat about the nits
No description provided.