Not sure if it is purposeful, but this review is no longer available for viewing. Even if we upload a new patch or commit, this is still useful for people looking back at the patch review.
Evan On Wed, Oct 8, 2008 at 1:14 PM, <[EMAIL PROTECTED]> wrote: > Thanks for the thorough review. New file attached to the issue. > > > http://codereview.appspot.com/7441/diff/1/3 > File > > java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/RpcServlet.java > (right): > > http://codereview.appspot.com/7441/diff/1/3#newcode155 > Line 155: } > On 2008/10/07 23:44:13, johnfargo wrote: > >> space between methods and between members and ctor >> > > Done. > > http://codereview.appspot.com/7441/diff/1/2 > File > > java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/RpcServletTest.java > (right): > > http://codereview.appspot.com/7441/diff/1/2#newcode74 > Line 74: assertDoGetWithoutHandler(null, "function"); > On 2008/10/07 23:44:13, johnfargo wrote: > >> It looks like these don't actually perform the test you want -- you'll >> > need to > >> run EasyMock.verify(response) on the ServletResponse object you're >> > creating for > >> each test. >> >> You could probably unroll the test helper methods too, since they're >> > not > >> repeatedly used too much. Thus testDoGetNormal would look like: >> request = createMockRequest(req, callback); >> response = createExpectedResponse(string, code); >> expect(handler.stuffHappens); >> servlet.doGet(req, response); >> verify(response); >> > > Done. Should be all fixed now. > > http://codereview.appspot.com/7441/diff/1/2#newcode90 > Line 90: int httpStatusCode) throws Exception { > On 2008/10/07 23:44:13, johnfargo wrote: > >> args on the first line, wrapped 4-indented after >> > > Done. > > http://codereview.appspot.com/7441/diff/1/2#newcode97 > Line 97: EasyMock.replay(request, response, handler, writer); > On 2008/10/07 23:44:13, johnfargo wrote: > >> stylistically we import static on these eg. >> import static ...EasyMock.expect; >> > > Done. > > http://codereview.appspot.com/7441/diff/1/2#newcode106 > Line 106: int httpStatusCode) throws Exception { > On 2008/10/07 23:44:13, johnfargo wrote: > >> args on the first line, wrapped 4-indented after >> > > Done. > > http://codereview.appspot.com/7441/diff/1/2#newcode137 > Line 137: String contentType, int httpStatusCode) throws IOException { > On 2008/10/07 23:44:13, johnfargo wrote: > >> This can be called createHttpResponse (and perhaps used for POST tests >> > later) > > Done. > > > http://codereview.appspot.com/7441 >
