Defaults for JsonFactory and HttpTransport and use by GoogleCredential.

Defaults for JsonFactory and HttpTransport and use by GoogleCredential.

[beaton, 2014-03-26 01:14:15]

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
File
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java
(right):

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java:92:
defaultJsonFactory = new JacksonFactory();
do you want to do anything crazy like use reflection to avoid the direct
dependency?

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
File
google-api-client/src/main/java/com/google/api/client/googleapis/auth/oauth2/GoogleCredential.java
(right):

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
google-api-client/src/main/java/com/google/api/client/googleapis/auth/oauth2/GoogleCredential.java:260:
return fromStream(
indent 2 spaces

[Anthony Moore, 2014-03-27 00:00:36]

Fixed a code comment, applied one suggestion and responded to suggestion about
reflection.

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
File
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java
(right):

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java:92:
defaultJsonFactory = new JacksonFactory();
On 2014/03/26 01:14:15, beaton wrote:
> do you want to do anything crazy like use reflection to avoid the direct
> dependency?

I thought about this, but I would still lean towards making this a conventional
compile-time dependency. We don't have a simple fallback to make it work if it
can't be loaded, and this means a developer is more likely to discover the
dependency at build time than run time. Java's late resolution of types should
mean that this only affect run-time behavior if you trigger this code path.

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
File
google-api-client/src/main/java/com/google/api/client/googleapis/auth/oauth2/GoogleCredential.java
(right):

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
google-api-client/src/main/java/com/google/api/client/googleapis/auth/oauth2/GoogleCredential.java:260:
return fromStream(
On 2014/03/26 01:14:15, beaton wrote:
> indent 2 spaces

Done.

[beaton, 2014-03-27 02:01:22]

LGTM.

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
File
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java
(right):

https://codereview.appspot.com/79890047/diff/1/google-api-client/src/main/jav...
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java:92:
defaultJsonFactory = new JacksonFactory();
That makes sense.  You might want to add a comment to the class explaining the
choices about dependencies here.

[Anthony Moore, 2014-03-27 20:43:31]

Processed the last comment, fix a findbugs issue with threading and applied the
Beta attribute. Note that I rebased from the dependent change, so the relative
deltas are inaccurate, but the absolute delta is OK.

[Orest, 2014-03-28 16:39:54]

LGTM

Just some nits.

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/main...
File
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java
(right):

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/main...
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java:105:
* {@link Beta} <br/>
This line seems to be indented one space too much.

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/test...
File
google-api-client/src/test/java/com/google/api/client/googleapis/GoogleUtilsTest.java
(right):

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/test...
google-api-client/src/test/java/com/google/api/client/googleapis/GoogleUtilsTest.java:66:
public static Map<String, String> parseQuery(String query) throws IOException {
I think we decided in another cl that this method does not belong here.

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/test...
google-api-client/src/test/java/com/google/api/client/googleapis/GoogleUtilsTest.java:81:

Do we need this new line here?

[Anthony Moore, 2014-04-02 00:21:39]

Fixed Open Comments.

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/main...
File
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java
(right):

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/main...
google-api-client/src/main/java/com/google/api/client/googleapis/GoogleUtils.java:105:
* {@link Beta} <br/>
On 2014/03/28 16:39:54, Orest wrote:
> This line seems to be indented one space too much.

Done.

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/test...
File
google-api-client/src/test/java/com/google/api/client/googleapis/GoogleUtilsTest.java
(right):

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/test...
google-api-client/src/test/java/com/google/api/client/googleapis/GoogleUtilsTest.java:66:
public static Map<String, String> parseQuery(String query) throws IOException {
On 2014/03/28 16:39:54, Orest wrote:
> I think we decided in another cl that this method does not belong here.
Yes, it will be gone, but it is a change list not in sync with this one.

https://codereview.appspot.com/79890047/diff/40001/google-api-client/src/test...
google-api-client/src/test/java/com/google/api/client/googleapis/GoogleUtilsTest.java:81:

On 2014/03/28 16:39:54, Orest wrote:
> Do we need this new line here?

Done.

[Orest, 2014-04-02 23:41:41]

LGTM

[wonderfly, 2014-04-04 22:15:49]

LGTM

[beaton, 2014-04-04 22:36:43]

Let's hold off on submitting this.  I want to resolve the conflict between what
eijdenberg@ has been asking for vs what is implemented here.