Re: [xwiki-devs] [VOTE] REST API Changes not passing CLIRR. Ignore errors

On 08/21/2012 04:02 PM, Ludovic Dubost wrote: Yes.

> Ludovic > > 2012/8/21 Sergiu Dumitriu <sergiu(a)xwiki.com>om>: >> On 08/21/2012 08:18 AM, Fabio Mancinelli wrote: >>> >>> On Fri, Jul 27, 2012 at 10:12 AM, Ludovic Dubost <ludovic(a)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. >> >> >> +1 as well. >> >> >>>> 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. >>> >>> 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...bd49bcc84e1dec3d…) >>> >>> 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...bd49bcc84e1dec3d… >>> * null, false in methods like this >>> >>> https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d… >>> . 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...bd49bcc84e1dec3d…) >>> >>> 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. >> >> >> I'd rather not keep the old methods, but since this is a public class, it >> needs to follow our deprecation strategy, so +1 for Fabio's suggestion.