Handling post requests for Accel servlet (issue1822041)

2010-07-12 Thread gagan . goku
Reviewers: johnfargo, zhoresh, shindig.remailer_gmail.com, dev-remailer_shindig.apache.org, cool-shindig-committers_googlegroups.com, Please review this at http://codereview.appspot.com/1822041/show Affected files: main/java/org/apache/shindig/gadgets/http/BasicHttpFetcher.java main/java

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-12 Thread gagan . goku
adding tests http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-12 Thread gagan . goku
beautification http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-13 Thread anupama . dutta
http://codereview.appspot.com/1822041/diff/9001/10004 File main/java/org/apache/shindig/gadgets/uri/UriUtils.java (right): http://codereview.appspot.com/1822041/diff/9001/10004#newcode70 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:70: // Headers that the fetcher itself would like to f

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-13 Thread gagan . goku
http://codereview.appspot.com/1822041/diff/9001/10004 File main/java/org/apache/shindig/gadgets/uri/UriUtils.java (right): http://codereview.appspot.com/1822041/diff/9001/10004#newcode70 main/java/org/apache/shindig/gadgets/uri/UriUtils.java:70: // Headers that the fetcher itself would like to f

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-13 Thread gagan . goku
addressing comments http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-13 Thread gagan . goku
reverting basichttpfetcher http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-13 Thread gagan . goku
fixing comment http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-14 Thread gagan . goku
setting follow redirect to false for accel http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-14 Thread johnfargo
Somehow the patch got out of sync w/ the current Shindig (chunk mismatch) -- give it a quick update. Thx http://codereview.appspot.com/1822041/diff/33001/34001 File main/java/org/apache/shindig/gadgets/http/HttpRequest.java (right): http://codereview.appspot.com/1822041/diff/33001/34001#newcode

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-14 Thread gagan . goku
http://codereview.appspot.com/1822041/diff/33001/34001 File main/java/org/apache/shindig/gadgets/http/HttpRequest.java (right): http://codereview.appspot.com/1822041/diff/33001/34001#newcode54 main/java/org/apache/shindig/gadgets/http/HttpRequest.java:54: // TODO: Convert to Map> to allow multip

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-14 Thread gagan . goku
addressing comments http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-15 Thread John Hjelmstad
Hey Gagan: Thanks for the responses. Brief comments inline, then continuing the review. On Wed, Jul 14, 2010 at 11:20 PM, wrote: > > http://codereview.appspot.com/1822041/diff/33001/34001 > File main/java/org/apache/shindig/gadgets/http/HttpRequest.java (right): > > http://codereview.appspot.co

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-16 Thread gagan . goku
But there could be multiple header values for the same header, like: Set-Cookie: NAME=value; domain=hello Set-Cookie: NAME2=value2; domain=buffalo Set header would just retain the last one (i hope thats why HttpServletResponse has addHeader and setHeader functions) Agreed w/ the rationale;

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-16 Thread gagan . goku
addressing comments http://codereview.appspot.com/1822041/show

Re: Handling post requests for Accel servlet (issue1822041)

2010-07-19 Thread John Hjelmstad
LGTM, looks great! Patch applied, tested, and committed. On Fri, Jul 16, 2010 at 12:57 PM, wrote: > addressing comments > > > http://codereview.appspot.com/1822041/show >