http://codereview.appspot.com/217091/diff/1/3 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java (right):
http://codereview.appspot.com/217091/diff/1/3#newcode73 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:73: List<ConcatUri> makeConcatUri(Gadget gadget, List<List<Uri>> resourceUris, I am not clear why the interface here need list of lists, as encapsulation go I don't think the manager need that detail. http://codereview.appspot.com/217091/diff/1/3#newcode84 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:84: public static final class ConcatUri { add class javadoc that explain snippet http://codereview.appspot.com/217091/diff/1/3#newcode103 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:103: public static final class ValidatedUri extends ProxyUriBase { Where is ProxyUriBase? http://codereview.appspot.com/217091/diff/1/4 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java (right): http://codereview.appspot.com/217091/diff/1/4#newcode49 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:49: public DefaultConcatUriManager(ContainerConfig config, Versioner versioner) { Don't you need to specify versioner as nullable? http://codereview.appspot.com/217091/diff/1/4#newcode73 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:73: // Above params are common for all generated Uris. Since it is common, it sounds like a conversion that can be extracted to a utility class or a base class for all UriManagers http://codereview.appspot.com/217091/diff/1/4#newcode85 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:85: String version = versions != null ? versions.get(i++) : null; Maybe iterator on versions instead of index http://codereview.appspot.com/217091/show
