Hey Ziv: Just a few little cleanups, which I've already done myself and committed. Thanks!
--John http://codereview.appspot.com/186252/diff/4001/4011 File features/src/main/javascript/features/features.txt (right): http://codereview.appspot.com/186252/diff/4001/4011#newcode33 features/src/main/javascript/features/features.txt:33: features/core.none/feature.xml out of order http://codereview.appspot.com/186252/diff/4001/4005 File java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java (right): http://codereview.appspot.com/186252/diff/4001/4005#newcode72 java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java:72: HtmlAccelServlet.ACCEL_GADGET_PARAM_VALUE) { we should abstract this into its own (static) method on HtmlAccelServlet (which, for dependency reasons, could be switched out to a helper file if needed later) http://codereview.appspot.com/186252/diff/4001/4009 File java/gadgets/src/main/java/org/apache/shindig/gadgets/render/GadgetRewritersProvider.java (right): http://codereview.appspot.com/186252/diff/4001/4009#newcode51 java/gadgets/src/main/java/org/apache/shindig/gadgets/render/GadgetRewritersProvider.java:51: HtmlAccelServlet.ACCEL_GADGET_PARAM_VALUE) { helper used here http://codereview.appspot.com/186252/diff/4001/4006 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java (right): http://codereview.appspot.com/186252/diff/4001/4006#newcode79 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java:79: private static class AccelRewritersProvider implements Provider<List<GadgetRewriter>> { looks fine that this is written as such, though @Provides methods would work just as well http://codereview.appspot.com/186252/diff/4001/4006#newcode82 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/RewriteModule.java:82: @SuppressWarnings("unused") is this needed? http://codereview.appspot.com/186252/diff/4001/4010 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java (right): http://codereview.appspot.com/186252/diff/4001/4010#newcode79 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java:79: + "</Module>\n"; we can just change these two to use String.format(tpl, htmlEscaped(data)); http://codereview.appspot.com/186252/diff/4001/4010#newcode85 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java:85: public void setRequestPipeLine(RequestPipeline pipeline) { nit: lower-case l http://codereview.appspot.com/186252/diff/4001/4010#newcode117 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java:117: !data.getHeader(CONTENT_TYPE).contains(HTML_CONTENT)) { perhaps factor this into a little helper method http://codereview.appspot.com/186252/diff/4001/4010#newcode180 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/HtmlAccelServlet.java:180: !CONTENT_LENGTH.equals(entry.getKey())) { I think we can get rid of these. For each we should set them ourselves. http://codereview.appspot.com/186252/diff/4001/4003 File java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java (right): http://codereview.appspot.com/186252/diff/4001/4003#newcode59 java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java:59: public void testHtmlAccelNoData() throws Exception { stylistically, people usually remove the "test" prefix with jUnit4 tests since they're already annotated. Not a big deal though. http://codereview.appspot.com/186252/diff/4001/4003#newcode106 java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java:106: expect(request.getRequestURL()).andReturn(new StringBuffer("gmodules.com/gadgets/accel")).once();
100 char
http://codereview.appspot.com/186252/diff/4001/4003#newcode144 java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/HtmlAccelServletTest.java:144: expect(request.getRequestURL()).andReturn(new StringBuffer("gmodules.com/gadgets/accel")).once(); same http://codereview.appspot.com/186252/show
