Already committed this - retrying send of responses and accommodations.
http://codereview.appspot.com/218057/diff/1/21 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java (right): http://codereview.appspot.com/218057/diff/1/21#newcode174 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java:174: this.includes = getMatchBundle(defaultInclude, Good observation. Added. On 2010/02/23 00:29:38, zhoresh wrote:
The original code "normalized" (trim spaces) the different paramaters,
was that
removed intentionally?
http://codereview.appspot.com/218057/diff/1/21#newcode229 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java:229: // gadget spec if !onlyAllowExcludes. Also good note. This no longer applies (comment was copied from previous impl). I've removed the comment, since the gadget spec can override the excludes whenever it wants. Makes me wonder if that's actually a bug -- the existing test cases were sort of unclear on this. It seems intuitive and reasonable to me for exclude overrides to be accepted when specified by the gadget spec in any case. On 2010/02/23 00:29:38, zhoresh wrote:
This comment should apply to include not exclude?
http://codereview.appspot.com/218057/diff/1/21#newcode370 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java:370: public boolean isSplitJsEnabled() { Side-effect of this being a CL split from a gigantic one that includes a lot of other changes. I've added some basic test cases for this trivial value. On 2010/02/23 00:29:38, zhoresh wrote:
Why is it part of content rewriter now? Also add simple test case for this
http://codereview.appspot.com/218057/diff/1/10 File java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BaseRewriterTestCase.java (right): http://codereview.appspot.com/218057/diff/1/10#newcode62 java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BaseRewriterTestCase.java:62: protected Set<String> tags; Yes and no :) No in this CL, yes in several others that will follow. On 2010/02/23 00:29:38, zhoresh wrote:
Is this tags still needed?
http://codereview.appspot.com/218057/show
