atom feed19 messages in org.xwiki.devsRe: [xwiki-devs] [VOTE] REST API Chan...
FromSent OnAttachments
Ludovic DubostJul 27, 2012 1:12 am 
Ludovic DubostAug 7, 2012 10:33 pm 
Fabio MancinelliAug 21, 2012 5:18 am 
Sergiu DumitriuAug 21, 2012 6:19 am 
Ludovic DubostAug 21, 2012 1:02 pm 
Vincent MassolAug 21, 2012 1:17 pm 
Fabio MancinelliAug 21, 2012 1:48 pm 
Sergiu DumitriuAug 21, 2012 1:56 pm 
Vincent MassolAug 21, 2012 2:05 pm 
Ludovic DubostAug 21, 2012 3:54 pm 
Thomas MortagneAug 25, 2012 8:40 am 
Ludovic DubostOct 8, 2012 7:35 am 
Ludovic DubostOct 10, 2012 7:37 am 
Fabio MancinelliOct 12, 2012 8:59 am 
Jerome VelociterOct 12, 2012 11:09 am 
Thomas MortagneOct 14, 2012 1:55 am 
Fabio MancinelliOct 17, 2012 2:14 pm 
Fabio MancinelliOct 19, 2012 11:24 am 
Ludovic DubostOct 19, 2012 1:04 pm 
Subject:Re: [xwiki-devs] [VOTE] REST API Changes not passing CLIRR. Ignore errors
From:Ludovic Dubost (ludo@xwiki.com)
Date:Aug 21, 2012 1:02:40 pm
List:org.xwiki.devs

So we should add methods to keep the old methods available and mark all of them/the whole class deprecated ?

Ludovic

2012/8/21 Sergiu Dumitriu <ser@xwiki.com>:

On 08/21/2012 08:18 AM, 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.

+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...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.

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.