On Fri, Oct 12, 2012 at 8:09 PM, Jerome Velociter <[email protected]> wrote: > On 10/12/2012 05:59 PM, Fabio Mancinelli wrote: >> >> On Mon, Oct 8, 2012 at 4:35 PM, Ludovic Dubost <[email protected]> wrote: >>> >>> Hi, >>> >>> All votes told to move the code to internal packages. >>> >>> This is something I can do, however I'm not sure exactly which code >>> should go into the internal package and which one should stay where it >>> is. >>> >>> Right now here is the list of classes failing or not the CLIRR tests >>> >>> Not failing: >>> >>> org.xwiki.rest.ComponentsObjectFactory >>> org.xwiki.rest.Constants >>> org.xwiki.rest.RangeIterable >>> org.xwiki.rest.Relations >>> org.xwiki.rest.Utils >>> org.xwiki.rest.XWikiAuthentication >>> org.xwiki.rest.XWikiJaxRsApplication >>> org.xwiki.rest.XWikiResource >>> org.xwiki.rest.XWikiRestComponent >>> org.xwiki.rest.XWikiRestletJaxRsApplication >>> org.xwiki.rest.XWikiRestletServlet >>> org.xwiki.rest.XWikiSecretVerifier >>> org.xwiki.rest.XWikiSetupCleanupFilter >>> org.xwiki.rest.exceptions.QueryExceptionMapper >>> org.xwiki.rest.exceptions.XWikiExceptionMapper >>> org.xwiki.rest.representations.TextPlainReader >>> org.xwiki.rest.representations.comments.FormUrlEncodedCommentReader >>> org.xwiki.rest.representations.comments.TextPlainCommentReader >>> org.xwiki.rest.representations.objects.FormUrlEncodedObjectReader >>> org.xwiki.rest.representations.objects.FormUrlEncodedPropertyReader >>> org.xwiki.rest.representations.objects.TextPlainPropertyReader >>> org.xwiki.rest.representations.pages.FormUrlEncodedPageReader >>> org.xwiki.rest.representations.pages.TextPlainPageReader >>> org.xwiki.rest.representations.tags.FormUrlEncodedTagsReader >>> org.xwiki.rest.representations.tags.TextPlainTagsReader >>> org.xwiki.rest.resources.BrowserAuthenticationResource >>> org.xwiki.rest.resources.RootResource >>> org.xwiki.rest.resources.SyntaxesResource >>> org.xwiki.rest.resources.attachments.AttachmentAtPageVersionResource >>> org.xwiki.rest.resources.attachments.AttachmentHistoryResource >>> org.xwiki.rest.resources.attachments.AttachmentResource >>> org.xwiki.rest.resources.attachments.AttachmentVersionResource >>> org.xwiki.rest.resources.classes.ClassPropertiesResource >>> org.xwiki.rest.resources.classes.ClassPropertyResource >>> org.xwiki.rest.resources.classes.ClassResource >>> org.xwiki.rest.resources.classes.ClassesResource >>> org.xwiki.rest.resources.objects.BaseObjectsResource >>> org.xwiki.rest.resources.pages.ModifiablePageResource >>> org.xwiki.rest.resources.pages.PageTagsResource >>> org.xwiki.rest.resources.pages.PageTranslationsResource >>> org.xwiki.rest.resources.spaces.SpaceResource >>> org.xwiki.rest.resources.spaces.SpacesResource >>> org.xwiki.rest.resources.tags.TagsResource >>> org.xwiki.rest.resources.wikis.WikiPagesResource >>> org.xwiki.rest.resources.wikis.WikiSearchQueryResource >>> org.xwiki.rest.resources.wikis.WikisResource >>> >>> Failing: >>> >>> org.xwiki.rest.DomainObjectFactory >>> org.xwiki.rest.resources.BaseAttachmentsResource >>> org.xwiki.rest.resources.BaseSearchResult >>> org.xwiki.rest.resources.ModificationsResource >>> org.xwiki.rest.resources.attachments.AttachmentsAtPageVersionResource >>> org.xwiki.rest.resources.attachments.AttachmentsResource >>> org.xwiki.rest.resources.comments.CommentResource >>> org.xwiki.rest.resources.comments.CommentVersionResource >>> org.xwiki.rest.resources.comments.CommentsResource >>> org.xwiki.rest.resources.comments.CommentsVersionResource >>> org.xwiki.rest.resources.objects.AllObjectsForClassNameResource >>> org.xwiki.rest.resources.objects.ObjectAtPageVersionResource >>> org.xwiki.rest.resources.objects.ObjectPropertiesAtPageVersionResource >>> org.xwiki.rest.resources.objects.ObjectPropertiesResource >>> org.xwiki.rest.resources.objects.ObjectPropertyAtPageVersionResource >>> org.xwiki.rest.resources.objects.ObjectPropertyResource >>> org.xwiki.rest.resources.objects.ObjectResource >>> org.xwiki.rest.resources.objects.ObjectsAtPageVersionResource >>> org.xwiki.rest.resources.objects.ObjectsForClassNameResource >>> org.xwiki.rest.resources.objects.ObjectsResource >>> org.xwiki.rest.resources.pages.PageChildrenResource >>> org.xwiki.rest.resources.pages.PageHistoryResource >>> org.xwiki.rest.resources.pages.PageResource >>> org.xwiki.rest.resources.pages.PageTranslationHistoryResource >>> org.xwiki.rest.resources.pages.PageTranslationResource >>> org.xwiki.rest.resources.pages.PageTranslationVersionResource >>> org.xwiki.rest.resources.pages.PageVersionResource >>> org.xwiki.rest.resources.pages.PagesResource >>> org.xwiki.rest.resources.spaces.SpaceAttachmentsResource >>> org.xwiki.rest.resources.spaces.SpaceSearchResource >>> org.xwiki.rest.resources.tags.PagesForTagsResource >>> org.xwiki.rest.resources.wikis.WikiAttachmentsResource >>> org.xwiki.rest.resources.wikis.WikiSearchResource >>> >>> >>> So which one of these are not internal ? >>> >> I think that everything except XWikiResource and Relations should be >> moved to internal. >> These are implementations of the HTTP actions on different resources >> and the implementation of content-type conversions. Everything that is >> actually internal. >> >> Developers who wants to implement new resources only needs >> XWikiRestComponent, XWikiResource (which provides an extensible class >> with some utility methods) and XWikiRelations to have a list of the >> relations that can be specified in <link>s >> >> So I am going to move everything else in internal. > > > Sounds good, +1
+1 too > > Jerome > >> >> -Fabio >> >> >> >> >>> Ludovic >>> >>> 2012/8/25 Thomas Mortagne <[email protected]>: >>>> >>>> On Tue, Aug 21, 2012 at 10:18 PM, Vincent Massol <[email protected]> >>>> wrote: >>>>> >>>>> On Aug 21, 2012, at 2:18 PM, Fabio Mancinelli wrote: >>>>> >>>>>> On Fri, Jul 27, 2012 at 10:12 AM, Ludovic Dubost <[email protected]> >>>>>> 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 >>>>> [email protected] >>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>> >>>> >>>> >>>> -- >>>> Thomas Mortagne >>>> _______________________________________________ >>>> devs mailing list >>>> [email protected] >>>> http://lists.xwiki.org/mailman/listinfo/devs >>> >>> >>> >>> -- >>> Ludovic Dubost >>> Founder and CEO >>> Blog: http://blog.ludovic.org/ >>> XWiki: http://www.xwiki.com >>> Skype: ldubost GTalk: ldubost >>> _______________________________________________ >>> devs mailing list >>> [email protected] >>> http://lists.xwiki.org/mailman/listinfo/devs >> >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs -- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
