Thanks for taking a look. Ill take a stab at decoupling the enum binding after commit Ill start a separate thread about whether we want to keep the JSON-LIB code around.
On Mon, Feb 9, 2009 at 4:52 PM, Ian Boston <[email protected]> wrote: > Ok if there can be a small amount of post commit re-factor, I am definitely > +1 on this, big improvement. > Ian > > On 10 Feb 2009, at 00:47, [email protected] wrote: > > I definitely agree that xstream and json-lib should be removed if not in >> use (though I believe that xstream is being used for atom support >> currently??) >> >> I'd also be fine moving Enum back into social though it requires some >> work in the existing BeanConverters Id rather avoid in this CL. >> >> >> 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)); >> On 2009/02/09 23:48:03, ianboston wrote: >> >>> I think this creates a cyclic binding between social and common, is >>> >> that a good >> >>> idea ? >>> >> >> I wouldnt call it cyclic but I agree its not exactly ideal either. >> Multi-binding in Guice 2 would solve all ills here but until then.... >> >> 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> >> On 2009/02/09 23:48:03, ianboston wrote: >> >>> 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. >>> >> >> Mostly agreed though the notion encapsulated here isnt specific to >> opensocial even though all its current uses are. It needs to remain in >> common because of how our hard-coded bean conversion works. Im sure that >> can be fixed later I just dont want to overload this excessively. >> >> http://codereview.appspot.com/14066 >> > >
