Hi Ramesh, > Suggestions > 1) Is there is any strong reason to have both "common-api" and "common-core". > Can me move these packages into one module like "common-core"? for client and > server api/impl makes sense as someone could provide an alternative > implementation.
The separation between API (“api”) and implementation (“core”) was intentionally that it is clearly evident which parts are “visible” to an user and must be backward capable. In Olingo V2 this concept helped a lot to achieve that an update of the library does not lead to compilation errors. For that reason I would prefer to keep the separation. > > 2) Can we rename " commons-api ==>org.apache.olingo.commons.api.edm.provider" > to " commons-api ==>org.apache.olingo.commons.api.edm.csdl" or " > org.apache.olingo.commons.api.csdl". Same goes to "edm.provider.annotation" > package. I agree with your suggestion and prefer “org.apache.olingo.commons.api.edm.csdl”. Furthermore I would suggest to rename “org.apache.olingo.commons.api.domain” to “org.apache.olingo.commons.api.client”, or better this should be moved to “org.apache.olingo.client.api.domain”. However... > > 3) Next confusing part is package "org.apache.olingo.commons.api.domain" > package in "commons-api" and its implementation in "common-core". This is > parallel package what looks like same/similar intentions as > "org.apache.olingo.commons.api.data". These are only used to serialize and > deserialize content in client modules. If they are only designed for client > they should have been (should be moved to) in the client, if not > "org.apache.olingo.commons.api.data" should not have existed to begin with. > It is quite possible I am not seeing the intent of this package too, so in > that case if you can explain that would be great. …this could be not that easy. As far as I knew is this “confusing part” a (legacy) result of the merge of the client contribution with the server contribution. I will check if its possible to move the “client-only” code from the “commons” into the “client” modules. Nevertheless I would prefer to do a merge back of the current state (of OLINGO-564) into the master branch. Best regards, Michael > > Thanks. > > Ramesh.. > > ----- Original Message ----- > >> Hi, > >> as proposed I renamed... >> org.apache.olingo.client.core.edm.xml >> [Client*] -> ClientCsdl* (ClientCsdlProperty) >> org.apache.olingo.commons.api.domain >> [OData*] -> Client* (ClientProperty) >> org.apache.olingo.commons.api.edm.provider >> [*] -> Csdl* (CsdlProperty) > >> and pushed it into feature branch OLINGO-564 ( >> https://git1-us-west.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564 >> ). > >> Feedback is welcome and if there are no objections I would merge it into >> master branch (at least tomorrow). > >> Best regards, >> Michael > >>> On 28 Apr 2015, at 13:03, Bolz, Michael < [email protected] > wrote: >> > >>> Hi, >> > >>> push to feature branch OLINGO-564 is done ( >>> https://git1-us-west.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564 >>> ). >> >>> If there are no objections I would merge it into master branch (at least >>> tomorrow). >> > >>> Best regards, >> >>> Michael >> > >>>> On 28 Apr 2015, at 08:17, Bolz, Michael < [email protected] > wrote: >>> >> > >>>> Hi, >>> >> > >>>> because there are no additional opinions and proposals I would start with >>>> the >>>> renaming on which Ramesh and Christian already agreed. >>> >> >>>> org.apache.olingo.commons.api.edm.provider >>> >> >>>> [*] -> Csdl* (CsdlProperty) >>> >> > >>>> I give feedback when it is done. >>> >> > >>>> Best regards, >>> >> >>>> Michael >>> >> > >>>>> On 27 Apr 2015, at 15:24, Bolz, Michael < [email protected] > wrote: >>>> >>> >> > >>>>> Hi, >>>> >>> >> > >>>>> I also agree with the suggestion for adding a prefix to the classes in >>>>> the >>>>> package: org.apache.olingo.commons.api.edm.provider >>>> >>> >> >>>>> But I’am not sure about the “Csdl”. >>>> >>> >> >>>>> However I currently do not have a better proposal. >>>> >>> >> >>>>> Perhaps somebody has a better name. >>>> >>> >> > >>>>> Another naming issue I see is the prefix “OData" of the classes within >>>>> the >>>>> “org.apache.olingo.commons.api.domain” package. >>>> >>> >> >>>>> For me it seems that the prefix should be something like “Client” >>>>> because >>>>> the >>>>> classes are mainly (only) used in the context of the ODataClient. >>>> >>> >> > >>>>> Some opinions/suggestions about these points? >>>> >>> >> > >>>>> A result of the renaming could than look like: >>>> >>> >> > >>>>> org.apache.olingo.commons.api.data >>>> >>> >> >>>>> No changes; still: * (Property) >>>> >>> >> >>>>> org.apache.olingo.commons.api.domain >>>> >>> >> >>>>> [OData*] -> Client* (ClientProperty) >>>> >>> >> >>>>> org.apache.olingo.commons.api.edm >>>> >>> >> >>>>> No changes; still: Edm* (EdmProperty) >>>> >>> >> >>>>> org.apache.olingo.commons.api.edm.provider >>>> >>> >> >>>>> [*] -> Csdl* (CsdlProperty) >>>> >>> >> > >>>>> Best regards, Michael >>>> >>> >> > >>>>>> On 27 Apr 2015, at 08:20, Bolz, Michael < [email protected] > >>>>>> wrote: >>>>> >>>> >>> >> > >>>>>> Hi, >>>>> >>>> >>> >> > >>>>>> Thanks for feedback, I will take a look into your suggestion below >>>>>> and >>>>>> give >>>>>> feedback. >>>>> >>>> >>> >> >>>>>> If all look good I would do the merge with the master branch >>>>>> today/tomorrow. >>>>> >>>> >>> >> > >>>>>> Best regards, >>>>> >>>> >>> >> >>>>>> Michael >>>>> >>>> >>> >> > >>>>>>> On 24 Apr 2015, at 15:06, Ramesh Reddy < [email protected] > >>>>>>> wrote: >>>>>> >>>>> >>>> >>> >> > >>>>>>> looking good so far. How about this suggestion from before, >>>>>>> Christian >>>>>>> also >>>>>>> seemed to agree with it >>>>>> >>>>> >>>> >>> >> > >>>>>>> org.apache.ol in go.commons.api.edm.provider ==> objects created >>>>>>> dur >>>>>>> in >>>>>>> g >>>>>>> CSDL document pars in g. "Edm" would have been right prefix for >>>>>>> this, >>>>>>> s >>>>>>> in >>>>>>> ce can not be used how about "Csdl"? They represent objects from >>>>>>> this >>>>>>> document. >>>>>> >>>>> >>>> >>> >> > >>>>>>> After this I will take another look at them, give you feedback. >>>>>> >>>>> >>>> >>> >> > >>>>>>> Ramesh.. >>>>>> >>>>> >>>> >>> >>
smime.p7s
Description: S/MIME cryptographic signature
