On Tue, Aug 21, 2012 at 10:18 PM, Vincent Massol <vinc...@massol.net> wrote: > > On Aug 21, 2012, at 2:18 PM, Fabio Mancinelli wrote: > >> On Fri, Jul 27, 2012 at 10:12 AM, Ludovic Dubost <ludo...@xwiki.com> wrote: >>> As part of rest improvements to display pretty names of users and >>> other improvements, I'm getting CLIRR errors because of API changes of >>> the model and of public class: >>> >>> >>> 1/ Model CLIRR error because the version field has been moved to >>> PageSummary from Page. Page extends PageSummary. I need the version >>> field also in representations sending back only PageSummaries. >>> Unfortunately CLIRR does not realize that the version field is still >>> there when moved to the super class. I believe it's safe to ignore >>> this error. Howerver I've put ignore all errors on the Page class as I >>> don't have a way to ignore this specific error >>> >> Yep, I think it's safe. We're adding stuff in a representation (page >> summary) and keeping it also in the other, so API-wise it's ok. > > Note that CLIRR doesn't have false positives so if it complains it means > there's a breakage. The only decision to take is whether it's an "acceptable" > breakage, i.e. we voluntarily break assuming that nobody was using it and > accept that a few users might get broken. > >>> 2/ CLIRR errors because of parameter additions to objects that are >>> used (I think) only internally by the REST server API. Here are the >>> errors: >>> >>> [ERROR] org.xwiki.rest.DomainObjectFactory: In method 'public >>> org.xwiki.rest.model.jaxb.Attachment >>> createAttachment(org.xwiki.rest.model.jaxb.ObjectFactory, >>> java.net.URI, com.xpn.xwiki.api.Attachment, java.lang.String, >>> java.lang.String)' the number of arguments has changed >> >> The DomainObjectFactory is actually a utility class that is used to >> build REST-model objects from XWiki-model objects. >> It has been created just to prevent code duplication in resource >> implementations. > > The question is whether this is supposed to be a SPI or not. > >> Now I think it's unlikely that somebody uses it outside the REST >> module (a quick grep confirmed this for platform). >> >> The only use case for a developer of a module to use this class is if >> she wants to return a REST-model object and build it using the utility >> methods. >> I think this is quite unlikely. >> >> AFAIU all parameters additions are about "pretty names" >> (https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d20b57e970c293a459524d2e4#L3L81) >> >> If we want to be conservative we might do the following: we can add >> the new methods and preserve the old ones making them call the new >> ones with default parameters. >> >> * false in methods like this >> https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d20b57e970c293a459524d2e4#L3L407 >> * null, false in methods like this >> https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d20b57e970c293a459524d2e4#L3L494 >> . This implies that in the new implementation the if statement should >> also check for null values (like in this case: >> https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d20b57e970c293a459524d2e4#L3R629) >> >> We could also think about whether continuing to keep this class in the >> public API. It could make sense but I think that nobody will ever use >> it so we can start to @deprecate it and eventually move it in internal >> packages. > > Based on what you say I'd say that these classes/methods were put public by > mistake and should all be moved to the internal package without going through > deprecation.
+1 for internal, if it's not supposed to be used outside of the module that's the definition of internal > > Of course this means no other other XWiki modules should use them either > since they're internal. > > Also this means that we don't provide any SPI either since SPI are > user-public. Is that what we want? > > Thanks > -Vincent > _______________________________________________ > devs mailing list > devs@xwiki.org > http://lists.xwiki.org/mailman/listinfo/devs -- Thomas Mortagne _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
