Cleaned up some duplication#38
Conversation
The legacy method was doing the exact same thing as addIfBodyExists. Now merged the two and added an override to support the different source of byte stream
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it! |
|
|
||
| /** | ||
| * @param urlRewriter Rewriter to use for request URLs | ||
| * @param urlRewriter Rewriter to use for request URLs |
There was a problem hiding this comment.
I'd prefer if you revert the formatting changes for now. This format doesn't match what I think we'd be likely to use in the future (google-java-format), and I'd rather handle that in a more coordinated matter.
There was a problem hiding this comment.
Makes a lot of sense! I was unaware of the google java format
|
|
||
| private static void addBodyIfExists(HttpURLConnection connection, Request<?> request, byte[] body) | ||
| throws IOException, AuthFailureError { | ||
| if (body != null) { |
There was a problem hiding this comment.
both the callers already check that body != null, so either remove this check here and rename this method "addBody" (my preference), or remove the checks above.
There was a problem hiding this comment.
Agreed, I will commit the changes when I'm done work!
|
CLAs look good, thanks! |
|
Thanks for the cleanup. I tweaked your change a bit to revert some extraneous formatting changes and fix the indenting on the new function, but otherwise it looks good. |
|
Sorry for the leftover formatting, I noticed it this morning but I did not wanted to fix it until I was done work. FYI the google java format formatter for intellij/android studio keeps that formatting. I don't know if this is a bug with the formatter or it was just not implemented yet, thought I would let you know. |
|
We haven't yet done a pass of the codebase under google-java-format or made it part of the contribution process. That's being tracked in #1. Until then, I'd just say to try and keep things as consistent with what's there as possible while minimizing extraneous changes. Thanks again. |
The legacy method was doing the exact same thing as
addIfBodyExists. Now merged the two and added an override to support the different source of byte stream. Also formatted the file.