Well done ! This will help the mobile app ! Envoyé de mon iPhone
Le 19 oct. 2012 à 20:24, Fabio Mancinelli <[email protected]> a écrit : > Hi, > > I finally merged the pull request. > > As an additional step I've moved all the REST components to internal packages. > To do so I added a CLIRR skip to the POM that should be removed on the > next release. > > I'll merge the XAR import and Wiki creation in the next release > because they need some additional work. > > Thanks, > Fabio > > > > On Wed, Oct 17, 2012 at 11:14 PM, Fabio Mancinelli > <[email protected]> wrote: >> Hi everybody >> >> On Wed, Oct 10, 2012 at 4:37 PM, Ludovic Dubost <[email protected]> wrote: >>> Following a discussion with Vincent, I've created the >>> >>> feature-restimprovements feature branch on the xwiki xwiki-platform github >>> and passed it to fabio who will: >>> >>> - integrate his own patches for wiki creation and xar upload >>> - move internal code to internal >>> >>> I've create a new pull request for this branch >>> https://github.com/xwiki/xwiki-platform/pull/71 >>> >>> The older pull request (https://github.com/xwiki/xwiki-platform/pull/56) >>> has been closed. >> I did quite a lot of work on this pull request with several changes, >> mostly related to keep things consistent and some bugfixes. >> >> It seems to me that the pull request is ready to be merged. The code >> is not complex, but since query building and option handling make it >> quite complicated it would be good if somebody could have a look at it >> before I merge it. >> >> I also wrote some integration tests to check that things are working >> fine: https://github.com/xwiki/xwiki-enterprise/pull/33 >> >> Thanks, >> Fabio >> >> >>> Ludovic >>> >>> 2012/10/8 Ludovic Dubost <[email protected]> >>> >>>> 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 ? >>>> >>>> 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 >>> >>> >>> >>> -- >>> 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
