Some comments, by no means a complete set. I am wondering about the json-lib configuration and if its still used. If not, then it should be removed from the code base. If it is, then IMHO the binding to model.Enum should be expressed in such a way as not to pull Enum into common. Although Enum itself is a simple and almost empty concept, I think its not a good idea to split the Opensocial model between jars (IMHO). It might be possible to use the same approach as has been used with the xstream configuration, where the configuration is inside the social jar, avoiding splitting the model to undo a cyclic binding. If xstream isn't used any more, that should also be removed entirely.
http://codereview.appspot.com/14066/diff/1/117 File java/common/src/main/java/org/apache/shindig/protocol/conversion/jsonlib/BaseJsonLibConfig.java (right): http://codereview.appspot.com/14066/diff/1/117#newcode67 Line 67: morpherRegistry.registerMorpher(new EnumMorpher(org.apache.shindig.protocol.model.Enum.Field.class)); I think this creates a cyclic binding between social and common, is that a good idea ? http://codereview.appspot.com/14066/diff/1/16 File java/common/src/main/java/org/apache/shindig/protocol/model/Enum.java (right): http://codereview.appspot.com/14066/diff/1/16#newcode22 Line 22: * http://www.opensocial.org/Technical-Resources/opensocial-spec-v081/opensocial-reference#opensocial.Enum</a> Strictly speaking this is part of the OpenSocial API and not part of common. Although the contents of Enum is small and fairly non specific. http://codereview.appspot.com/14066
