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