http://codereview.appspot.com/1949051/diff/5001/6001 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java (left):
http://codereview.appspot.com/1949051/diff/5001/6001#oldcode85 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ProxyingVisitor.java:85: On 2010/08/30 20:03:35, Paul Lindner wrote:
Is this @Inject strictly necessary? It appears that this is
instantiated
directly in ProxyingContentRewriter. I also doubt that guice supports
...
syntax for injection, we may require a List or Collection instead..
Nice catch. The @Inject was meaningless. Don't know what guice does for ... though maybe it just passes null http://codereview.appspot.com/1949051/diff/5001/6002 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ResourceMutateVisitor.java (right): http://codereview.appspot.com/1949051/diff/5001/6002#newcode84 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ResourceMutateVisitor.java:84: this.resourceTags = new HashMap<String, String>(); On 2010/08/30 20:03:35, Paul Lindner wrote:
Make this Immutable?
Done. http://codereview.appspot.com/1949051/diff/5001/6002#newcode89 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ResourceMutateVisitor.java:89: On 2010/08/30 20:03:35, Paul Lindner wrote:
Please add some javadoc here.
Done. http://codereview.appspot.com/1949051/diff/5001/6002#newcode115 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ResourceMutateVisitor.java:115: On 2010/08/30 20:03:35, Paul Lindner wrote:
Please add some javadoc here.
Done. http://codereview.appspot.com/1949051/diff/5001/6002#newcode132 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ResourceMutateVisitor.java:132: On 2010/08/30 20:03:35, Paul Lindner wrote:
Please add some javadoc here. Also can this be a Collection instead of List?
Done. Collection looks cleaner as well. http://codereview.appspot.com/1949051/
