Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2159)

Issue 6208085: Adding Iterable<?> and Enum support and fixing initialization problem in UriTemplate (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 10 months ago by rmistry
Modified:
11 years, 10 months ago
Reviewers:
yanivi
Base URL:
https://google-http-java-client.googlecode.com/hg/
Visibility:
Public.

Description

Fix for- http://code.google.com/p/google-http-java-client/issues/detail?id=118 : UriTemplate.expand should support Iterable<?> for list parameters. http://code.google.com/p/google-http-java-client/issues/detail?id=117 : UriTemplate COMPOSITE_PREFIXES init problem.

Patch Set 1 #

Patch Set 2 : Minor fixes #

Total comments: 18

Patch Set 3 : Addressing codereview comments #

Patch Set 4 : Minor fix #

Total comments: 10

Patch Set 5 : Addressing code review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -25 lines) Patch
M google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java View 1 2 3 4 7 chunks +28 lines, -10 lines 0 comments Download
M google-http-client/src/test/java/com/google/api/client/http/UriTemplateTest.java View 1 2 5 chunks +45 lines, -15 lines 0 comments Download

Messages

Total messages: 6
rmistry
11 years, 10 months ago (2012-05-21 14:23:11 UTC) #1
yanivi
http://codereview.appspot.com/6208085/diff/5/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java File google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java (right): http://codereview.appspot.com/6208085/diff/5/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java#newcode81 google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java:81: COMPOSITE_PREFIXES.put('+', CompositeOutput.PLUS); this is unmaintainable. how about just: CompositeOutput.values(); ...
11 years, 10 months ago (2012-05-22 12:39:42 UTC) #2
rmistry
http://codereview.appspot.com/6208085/diff/5/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java File google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java (right): http://codereview.appspot.com/6208085/diff/5/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java#newcode81 google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java:81: COMPOSITE_PREFIXES.put('+', CompositeOutput.PLUS); On 2012/05/22 12:39:42, yanivi wrote: > this ...
11 years, 10 months ago (2012-05-22 15:47:06 UTC) #3
yanivi
http://codereview.appspot.com/6208085/diff/8001/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java File google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java (right): http://codereview.appspot.com/6208085/diff/8001/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java#newcode312 google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java:312: } else if (value instanceof Iterator<?>) { remove the ...
11 years, 10 months ago (2012-05-22 16:09:50 UTC) #4
rmistry
http://codereview.appspot.com/6208085/diff/8001/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java File google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java (right): http://codereview.appspot.com/6208085/diff/8001/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java#newcode312 google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java:312: } else if (value instanceof Iterator<?>) { On 2012/05/22 ...
11 years, 10 months ago (2012-05-22 16:35:37 UTC) #5
yanivi
11 years, 10 months ago (2012-05-22 20:09:46 UTC) #6
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b