You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by jo...@gmail.com on 2010/06/21 19:07:40 UTC
Re: Redesign '/concat' servlet' - Fetch resources concurrently
(issue1681044)
Hi Vikas:
This change looks excellent. My comments are all formatting nits, which
I can take care of before committing. Thanks!
--j
http://codereview.appspot.com/1681044/diff/1/2
File
src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
(right):
http://codereview.appspot.com/1681044/diff/1/2#newcode83
src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:83:
nit: excess newline
http://codereview.appspot.com/1681044/diff/1/2#newcode172
src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:172:
new ArrayList<Pair<Uri, FutureTask<RequestContext>>>();
nit: 4-space indent
http://codereview.appspot.com/1681044/diff/1/2#newcode178
src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java:178:
new FutureTask<RequestContext>(new HttpFetchCallable(httpReq));
same
http://codereview.appspot.com/1681044/diff/1/3
File
src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java
(right):
http://codereview.appspot.com/1681044/diff/1/3#newcode61
src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java:61:
private final Executor SequentialExecutor = new Executor() {
nit: start w/ lowercase
http://codereview.appspot.com/1681044/diff/1/3#newcode68
src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java:68:
private final Executor ThreadedExecutor = new Executor() {
same
http://codereview.appspot.com/1681044/diff/1/3#newcode154
src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java:154:
assertEquals(200, recorder.getHttpStatusCode());
this method can be consolidated w/ the one above, with just an executor
passed in
http://codereview.appspot.com/1681044/show