You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by zh...@gmail.com on 2010/03/07 20:17:37 UTC
Re: StyleTagExtractorVisitor implementation (issue223098)
http://codereview.appspot.com/223098/diff/2001/2003
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleTagContentRewriter.java
(right):
http://codereview.appspot.com/223098/diff/2001/2003#newcode33
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleTagContentRewriter.java:33:
public class StyleTagContentRewriter extends DomWalker.Rewriter {
create tests
http://codereview.appspot.com/223098/diff/2001/2003#newcode55
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleTagContentRewriter.java:55:
new ConcatLinkRewriter.Css(config, concatUriManager));
Two comments:
- Class name consistency, one Visitor and one Rewriter.Css but both are
same interface
- New link tags that created by StyleTagExtractor are not being
concatenate. Consider using separate Dom Walkers (concat activated after
tag extractor)
http://codereview.appspot.com/223098/show
Re: StyleTagExtractorVisitor implementation (issue223098)
Posted by John Hjelmstad <jo...@gmail.com>.
On Sun, Mar 7, 2010 at 11:17 AM, <zh...@gmail.com> wrote:
>
> http://codereview.appspot.com/223098/diff/2001/2003
> File
>
>
> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleTagContentRewriter.java
> (right):
>
> http://codereview.appspot.com/223098/diff/2001/2003#newcode33
>
> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleTagContentRewriter.java:33:
> public class StyleTagContentRewriter extends DomWalker.Rewriter {
> create tests
>
IMO the [Gadget|Request|Content]Rewriter classes are a good place to put
integration tests, while VisitorTests will serve as unit testing. Thoughts?
Stub put in for the moment; new patch in a moment. I'll migrate the old
HTMLContentRewriter test (pieces) over in a separate CL.
>
> http://codereview.appspot.com/223098/diff/2001/2003#newcode55
>
> java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleTagContentRewriter.java:55:
> new ConcatLinkRewriter.Css(config, concatUriManager));
> Two comments:
> - Class name consistency, one Visitor and one Rewriter.Css but both are
> same interface
> - New link tags that created by StyleTagExtractor are not being
> concatenate. Consider using separate Dom Walkers (concat activated after
> tag extractor)
Renamed in previous CL -- will attach patch reflecting this shortly.
>
>
> http://codereview.appspot.com/223098/show
>