You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@olingo.apache.org by Ramesh Reddy <ra...@redhat.com> on 2015/04/16 17:40:35 UTC
Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Hi,
Since we are refactoring lot of common-api, common-core code, is it possible to rename classes in "org.apache.olingo.commons.api.edm.provider" package in the common-api module to prefix something like "Edm", so they are EdmAction,
EdmActionImport etc? I always get confused which classes to select when some of the classes with same name show up from "org.apache.olingo.commons.api.data" package. By adding this prefix, it would be much more clear IMO.
what do you guys think?
Ramesh..
http://teiid.org
Re: Renaming classes in "org.apache.olingo.*"
Posted by "Bolz, Michael" <mi...@sap.com>.
Hi together,
to prevent a possible bigger merge with the master and I did not read any objections
I merged the feature branch back into master.
See latest commit: https://git1-us-west.apache.org/repos/asf?p=olingo-odata4.git;a=commit;h=61500e685fca852fac301f473fcca8a2918f887e <https://git1-us-west.apache.org/repos/asf?p=olingo-odata4.git;a=commit;h=61500e685fca852fac301f473fcca8a2918f887e>
As always, feedback is welcome ;o)
Best regards,
Michael
> On 30 Apr 2015, at 06:15, Bolz, Michael <mi...@sap.com> wrote:
>
> Hi,
>
> I extracted the client related parts into to ³client-api/core² modules and
> updated feature branch OLINGO-564
> (https://git1-us-west.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h
> =refs/heads/OLINGO-564).
>
> As always: Feedback is welcome ;o)
>
> Best regards,
> Michael
>
>
>
> On 29/04/15 07:57, "Bolz, Michael" <mi...@sap.com> wrote:
>
>> 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
>
Re: Renaming classes in "org.apache.olingo.*"
Posted by "Bolz, Michael" <mi...@sap.com>.
Hi,
I extracted the client related parts into to ³client-api/core² modules and
updated feature branch OLINGO-564
(https://git1-us-west.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h
=refs/heads/OLINGO-564).
As always: Feedback is welcome ;o)
Best regards,
Michael
On 29/04/15 07:57, "Bolz, Michael" <mi...@sap.com> wrote:
>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
Re: Renaming classes in "org.apache.olingo.*"
Posted by "Bolz, Michael" <mi...@sap.com>.
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 < michael.bolz@sap.com > 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 < michael.bolz@sap.com > 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 < michael.bolz@sap.com > 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 < michael.bolz@sap.com >
>>>>>> 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 < rareddy@redhat.com >
>>>>>>> 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..
>>>>>>
>>>>>
>>>>
>>>
>>
Re: Renaming classes in "org.apache.olingo.*"
Posted by Ramesh Reddy <ra...@redhat.com>.
Looks good Michael.
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.
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.
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.
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 < michael.bolz@sap.com > 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 < michael.bolz@sap.com > 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 < michael.bolz@sap.com > 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 < michael.bolz@sap.com >
> > > > > 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 < rareddy@redhat.com >
> > > > > > 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..
> > > > >
> > > >
> > >
> >
>
Re: Renaming classes in "org.apache.olingo.*"
Posted by "Bolz, Michael" <mi...@sap.com>.
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 <mi...@sap.com> 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 <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 <michael.bolz@sap.com <ma...@sap.com>> 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 <michael.bolz@sap.com <ma...@sap.com>> 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 <michael.bolz@sap.com <ma...@sap.com>> 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 <rareddy@redhat.com <ma...@redhat.com>> 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..
>>>>>
>>>
>>
>
Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Posted by "Bolz, Michael" <mi...@sap.com>.
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 <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 <mi...@sap.com> 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 <mi...@sap.com> 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 <mi...@sap.com> 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 <ra...@redhat.com> 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..
>>>>
>>
>
Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Posted by "Bolz, Michael" <mi...@sap.com>.
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 <mi...@sap.com> 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 <mi...@sap.com> 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 <ra...@redhat.com> 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..
>>>
>
Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Posted by "Bolz, Michael" <mi...@sap.com>.
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 <mi...@sap.com> 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 <ra...@redhat.com> 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..
>>
Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Posted by "Bolz, Michael" <mi...@sap.com>.
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 <ra...@redhat.com> 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..
>
> ----- Original Message -----
>
>> Hi,
>
>> I updated the OLINGO-564 feature branch (
>> https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564
>> ).
>> Now all classes within "org/apache/olingo/client/core/edm/xml” are package
>> visible only (expect the “ClientEdmx” as entry point to xml parsing from
>> edm).
>> “ClientXMLMetadata” was moved from "org/apache/olingo/client/core/edm/xml” to
>> "org/apache/olingo/client/core/edm” because it is the entry point to xml
>> parsing from the outside of "org/apache/olingo/client/core/edm” (used in
>> “ClientODataDeserializerImpl”).
>
>> Feedback is welcome and I hope we can merge these changes into master before
>> the “beta-03” release.
>
>> Best regards,
>> Michael
>
>>> On 23 Apr 2015, at 16:08, Bolz, Michael < michael.bolz@sap.com > wrote:
>>
>
>>> Hi together,
>>
>
>>> based on your suggestions I created OLINGO-564 feature branch (
>>> https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564
>>> ).
>>
>>> I started there with the removal of ‘provider’, adding more ‘package-info’
>>> and renaming of client-core classes.
>>
>
>>>> On 21 Apr 2015, at 17:25, Ramesh Reddy < rareddy@redhat.com > wrote:
>>>
>>
>
>>>> +1 on removing ".provider". +1 on package private.
>>>
>>
>
>>> The “package private” modification is not that easy, because some classes
>>> (in
>>> org/apache/olingo/client/core/edm/xml/* <->
>>> org/apache/olingo/client/core/edm/xml/annotations) are dependent on each
>>> other.
>>
>>> Actual I check for a solution with probably removal of “annotations”
>>> package.
>>
>>> As soon I have an idea I will inform you.
>>
>
>>> Best regards,
>>
>>> Michael
>>
Re: Renaming classes in
"org.apache.olingo.commons.api.edm.provider"
Posted by Ramesh Reddy <ra...@redhat.com>.
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..
----- Original Message -----
> Hi,
> I updated the OLINGO-564 feature branch (
> https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564
> ).
> Now all classes within "org/apache/olingo/client/core/edm/xml” are package
> visible only (expect the “ClientEdmx” as entry point to xml parsing from
> edm).
> “ClientXMLMetadata” was moved from "org/apache/olingo/client/core/edm/xml” to
> "org/apache/olingo/client/core/edm” because it is the entry point to xml
> parsing from the outside of "org/apache/olingo/client/core/edm” (used in
> “ClientODataDeserializerImpl”).
> Feedback is welcome and I hope we can merge these changes into master before
> the “beta-03” release.
> Best regards,
> Michael
> > On 23 Apr 2015, at 16:08, Bolz, Michael < michael.bolz@sap.com > wrote:
>
> > Hi together,
>
> > based on your suggestions I created OLINGO-564 feature branch (
> > https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564
> > ).
>
> > I started there with the removal of ‘provider’, adding more ‘package-info’
> > and renaming of client-core classes.
>
> > > On 21 Apr 2015, at 17:25, Ramesh Reddy < rareddy@redhat.com > wrote:
> >
>
> > > +1 on removing ".provider". +1 on package private.
> >
>
> > The “package private” modification is not that easy, because some classes
> > (in
> > org/apache/olingo/client/core/edm/xml/* <->
> > org/apache/olingo/client/core/edm/xml/annotations) are dependent on each
> > other.
>
> > Actual I check for a solution with probably removal of “annotations”
> > package.
>
> > As soon I have an idea I will inform you.
>
> > Best regards,
>
> > Michael
>
Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Posted by "Bolz, Michael" <mi...@sap.com>.
Hi,
I updated the OLINGO-564 feature branch (https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564).
Now all classes within "org/apache/olingo/client/core/edm/xml” are package visible only (expect the “ClientEdmx” as entry point to xml parsing from edm).
“ClientXMLMetadata” was moved from "org/apache/olingo/client/core/edm/xml” to "org/apache/olingo/client/core/edm” because it is the entry point to xml
parsing from the outside of "org/apache/olingo/client/core/edm” (used in “ClientODataDeserializerImpl”).
Feedback is welcome and I hope we can merge these changes into master before the “beta-03” release.
Best regards,
Michael
> On 23 Apr 2015, at 16:08, Bolz, Michael <mi...@sap.com> wrote:
>
> Hi together,
>
> based on your suggestions I created OLINGO-564 feature branch (https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564 <https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564>).
> I started there with the removal of ‘provider’, adding more ‘package-info’ and renaming of client-core classes.
>
>> On 21 Apr 2015, at 17:25, Ramesh Reddy <rareddy@redhat.com <ma...@redhat.com>> wrote:
>>
>> +1 on removing ".provider". +1 on package private.
>
> The “package private” modification is not that easy, because some classes (in org/apache/olingo/client/core/edm/xml/* <-> org/apache/olingo/client/core/edm/xml/annotations) are dependent on each other.
> Actual I check for a solution with probably removal of “annotations” package.
> As soon I have an idea I will inform you.
>
> Best regards,
> Michael
>
Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Posted by "Bolz, Michael" <mi...@sap.com>.
Hi together,
based on your suggestions I created OLINGO-564 feature branch (https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564 <https://git-wip-us.apache.org/repos/asf?p=olingo-odata4.git;a=shortlog;h=refs/heads/OLINGO-564>).
I started there with the removal of ‘provider’, adding more ‘package-info’ and renaming of client-core classes.
> On 21 Apr 2015, at 17:25, Ramesh Reddy <ra...@redhat.com> wrote:
>
> +1 on removing ".provider". +1 on package private.
The “package private” modification is not that easy, because some classes (in org/apache/olingo/client/core/edm/xml/* <-> org/apache/olingo/client/core/edm/xml/annotations) are dependent on each other.
Actual I check for a solution with probably removal of “annotations” package.
As soon I have an idea I will inform you.
Best regards,
Michael
Re: Renaming classes in
"org.apache.olingo.commons.api.edm.provider"
Posted by Ramesh Reddy <ra...@redhat.com>.
Did you already close your refactoring JIRA? I thought work can be done under that one. Otherwise I can open one.
See lot of concepts in "edm" and "edm.provider" are similar/same, they both build metadata about service and service access, I did not understand why they were created as two separate interfaces, can you tell me reasons behind it? As you see, as a root apis they spread and cause the confusion in naming.
+1 on removing ".provider". +1 on package private.
Ramesh..
----- Original Message -----
> Hi Ramesh,
>
> sorry for my late response about this.
>
> common-api:
> Good Idea!
>
> common-core
> I chose this package name to show that this is an EDM implementation using
> the provider classes. Technically someone could implement the Edm
> Interfaces completely different. This is why I put them under
> core.edm.provider. The AbstractEdm.class in core.edm is independent from
> that and just handles caching. WDYT about this design? I could also live
> with deleting the .provider name from the package naming.
>
> Server-api/ server-core
> Sven was the one to implement the URI stuff so I don`t want to make
> assumptions there.
>
> Client-core:
> This is tricky. We have to use Jackson in the client code to provide an
> android implementation since the native xml parsers are not available on
> android devices. Since this is a given we extend the commons.api.provider
> classes to keep the commons module and the server module free from any
> Jackson dependencies. As you have stated these classes are verbose and I do
> not like that but I could not come up with a better solution than this.
> Although package private sounds like a very good idea!
>
> Would you like to create some JIRA issues for this so we can track the
> progress?
>
> Best Regards,
> Christian
>
> -----Original Message-----
> From: Ramesh Reddy [mailto:rareddy@redhat.com]
> Sent: Freitag, 17. April 2015 17:14
> To: dev@olingo.apache.org
> Subject: Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
>
> Hi Christian,
>
> Great, we are same page then. First of all, if packages can include a package
> documentation file to describe what they are used for, that would really
> good.
>
> I have not yet used "annotations" and "client" modules so not sure what those
> "domain" and "annotation" packages yet.
>
> common-api
> ----------------
>
> org.apache.olingo.commons.api.data ==> Shows all the Data objects of OData
> responses
>
> org.apache.olingo.commons.api.edm ==> Represents a EDM Objects created during
> the URI Parsing
>
> org.apache.olingo.commons.api.edm.provider ==> objects created during CSDL
> document parsing. "Edm" would have been right prefix for this, since can not
> be used how about "Csdl"? They represent objects from this document.
>
> common-core
> -----------------
> org.apache.olingo.commons.core.edm.provider ==> seems to extend classes from
> "common-api" package "org.apache.olingo.commons.api.edm" which itself is
> confusing, because I would have expected them in
> "org.apache.olingo.commons.core.edm" package not
> "org.apache.olingo.commons.api.edm.provider". These are so similar to
> provider classes, and used by URL parsing routines, gets confused with
> provider classes. So, at a minimum change the package the name here.
>
>
> server-api and server-core
> --------------------------
> I am not sure we really need interface abstraction really needed for those
> "uri" interfaces, but if somebody wants to develop a new server these will
> be useful.
>
> odata-client-core
> -------------------
> org.apache.olingo.client.core.edm.xml ==> This one always gets me. These are
> exactly same as the provider classes above, except that they have JSON
> deserialization code. I am not sure what is being provided by "jackson"
> library here, olingo practically provides each step of the parsing, its not
> like JAXB where the XML object created for based on data structure. So, for
> seem these classes seem verbose. Two things we can do. Either make them
> package private so that only framework classes have visibility for parsing
> the return response for client. Or move all the Deserializer code into
> single utility class and get rid of all classes (not sure this possible),
> otherwise do not depend on json library (jackson) based automatic parsing,
> as I mentioned above this is providing little or no automation in creation
> object. Also, there is no $metadata payload coming in JSON yet, what are
> these used for?
>
> Thank you.
>
> Ramesh..
>
> ----- Original Message -----
> > Hi Ramesh,
> >
> > naming is incredibly important in my opinion and I am always thankful for
> > any
> > kind of feedback on that.
> >
> > I had the same thoughts when creating the API and Provider classes. My
> > thought there was to have the edm provider beans without EDM prefix and the
> > EDM API with the prefix. I did not see that data beans at that time so
> > there
> > is a Property in the edm provider and a property in the data package.
> >
> > My first thought would be to prefix all Provider classes with "Prov" or
> > something like that but I did not have the time for that.
> >
> > If you have any other naming ideas please let me know.
> >
> > Best Regards,
> > Christian
> >
> > -----Original Message-----
> > From: Ramesh Reddy [mailto:rareddy@redhat.com]
> > Sent: Donnerstag, 16. April 2015 21:55
> > To: dev@olingo.apache.org
> > Subject: Re: Renaming classes in
> > "org.apache.olingo.commons.api.edm.provider"
> >
> > scratch that, I see some other classes with Edm prefix already. Surely as
> > you
> > can see even after looking at the code for sometime I still get confused
> > with similarly named classes. Renaming them to correctly represent their
> > function is really going help understand for service developers IMO. I
> > understand they are in different packages, but does not do whole lot of
> > good.
> >
> > I am still learning -:)
> >
> > Ramesh..
> >
> > ----- Original Message -----
> > > Hi,
> > >
> > > Since we are refactoring lot of common-api, common-core code, is it
> > > possible
> > > to rename classes in "org.apache.olingo.commons.api.edm.provider" package
> > > in
> > > the common-api module to prefix something like "Edm", so they are
> > > EdmAction,
> > > EdmActionImport etc? I always get confused which classes to select when
> > > some
> > > of the classes with same name show up from
> > > "org.apache.olingo.commons.api.data" package. By adding this prefix, it
> > > would be much more clear IMO.
> > >
> > > what do you guys think?
> > >
> > > Ramesh..
> > > http://teiid.org
> > >
> > >
> >
>
RE: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Posted by "Amend, Christian" <ch...@sap.com>.
Hi Ramesh,
sorry for my late response about this.
common-api:
Good Idea!
common-core
I chose this package name to show that this is an EDM implementation using the provider classes. Technically someone could implement the Edm Interfaces completely different. This is why I put them under core.edm.provider. The AbstractEdm.class in core.edm is independent from that and just handles caching. WDYT about this design? I could also live with deleting the .provider name from the package naming.
Server-api/ server-core
Sven was the one to implement the URI stuff so I don`t want to make assumptions there.
Client-core:
This is tricky. We have to use Jackson in the client code to provide an android implementation since the native xml parsers are not available on android devices. Since this is a given we extend the commons.api.provider classes to keep the commons module and the server module free from any Jackson dependencies. As you have stated these classes are verbose and I do not like that but I could not come up with a better solution than this. Although package private sounds like a very good idea!
Would you like to create some JIRA issues for this so we can track the progress?
Best Regards,
Christian
-----Original Message-----
From: Ramesh Reddy [mailto:rareddy@redhat.com]
Sent: Freitag, 17. April 2015 17:14
To: dev@olingo.apache.org
Subject: Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Hi Christian,
Great, we are same page then. First of all, if packages can include a package documentation file to describe what they are used for, that would really good.
I have not yet used "annotations" and "client" modules so not sure what those "domain" and "annotation" packages yet.
common-api
----------------
org.apache.olingo.commons.api.data ==> Shows all the Data objects of OData responses
org.apache.olingo.commons.api.edm ==> Represents a EDM Objects created during the URI Parsing
org.apache.olingo.commons.api.edm.provider ==> objects created during CSDL document parsing. "Edm" would have been right prefix for this, since can not be used how about "Csdl"? They represent objects from this document.
common-core
-----------------
org.apache.olingo.commons.core.edm.provider ==> seems to extend classes from "common-api" package "org.apache.olingo.commons.api.edm" which itself is confusing, because I would have expected them in "org.apache.olingo.commons.core.edm" package not "org.apache.olingo.commons.api.edm.provider". These are so similar to provider classes, and used by URL parsing routines, gets confused with provider classes. So, at a minimum change the package the name here.
server-api and server-core
--------------------------
I am not sure we really need interface abstraction really needed for those "uri" interfaces, but if somebody wants to develop a new server these will be useful.
odata-client-core
-------------------
org.apache.olingo.client.core.edm.xml ==> This one always gets me. These are exactly same as the provider classes above, except that they have JSON deserialization code. I am not sure what is being provided by "jackson" library here, olingo practically provides each step of the parsing, its not like JAXB where the XML object created for based on data structure. So, for seem these classes seem verbose. Two things we can do. Either make them package private so that only framework classes have visibility for parsing the return response for client. Or move all the Deserializer code into single utility class and get rid of all classes (not sure this possible), otherwise do not depend on json library (jackson) based automatic parsing, as I mentioned above this is providing little or no automation in creation object. Also, there is no $metadata payload coming in JSON yet, what are these used for?
Thank you.
Ramesh..
----- Original Message -----
> Hi Ramesh,
>
> naming is incredibly important in my opinion and I am always thankful for any
> kind of feedback on that.
>
> I had the same thoughts when creating the API and Provider classes. My
> thought there was to have the edm provider beans without EDM prefix and the
> EDM API with the prefix. I did not see that data beans at that time so there
> is a Property in the edm provider and a property in the data package.
>
> My first thought would be to prefix all Provider classes with "Prov" or
> something like that but I did not have the time for that.
>
> If you have any other naming ideas please let me know.
>
> Best Regards,
> Christian
>
> -----Original Message-----
> From: Ramesh Reddy [mailto:rareddy@redhat.com]
> Sent: Donnerstag, 16. April 2015 21:55
> To: dev@olingo.apache.org
> Subject: Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
>
> scratch that, I see some other classes with Edm prefix already. Surely as you
> can see even after looking at the code for sometime I still get confused
> with similarly named classes. Renaming them to correctly represent their
> function is really going help understand for service developers IMO. I
> understand they are in different packages, but does not do whole lot of
> good.
>
> I am still learning -:)
>
> Ramesh..
>
> ----- Original Message -----
> > Hi,
> >
> > Since we are refactoring lot of common-api, common-core code, is it
> > possible
> > to rename classes in "org.apache.olingo.commons.api.edm.provider" package
> > in
> > the common-api module to prefix something like "Edm", so they are
> > EdmAction,
> > EdmActionImport etc? I always get confused which classes to select when
> > some
> > of the classes with same name show up from
> > "org.apache.olingo.commons.api.data" package. By adding this prefix, it
> > would be much more clear IMO.
> >
> > what do you guys think?
> >
> > Ramesh..
> > http://teiid.org
> >
> >
>
Re: Renaming classes in
"org.apache.olingo.commons.api.edm.provider"
Posted by Ramesh Reddy <ra...@redhat.com>.
Hi Christian,
Great, we are same page then. First of all, if packages can include a package documentation file to describe what they are used for, that would really good.
I have not yet used "annotations" and "client" modules so not sure what those "domain" and "annotation" packages yet.
common-api
----------------
org.apache.olingo.commons.api.data ==> Shows all the Data objects of OData responses
org.apache.olingo.commons.api.edm ==> Represents a EDM Objects created during the URI Parsing
org.apache.olingo.commons.api.edm.provider ==> objects created during CSDL document parsing. "Edm" would have been right prefix for this, since can not be used how about "Csdl"? They represent objects from this document.
common-core
-----------------
org.apache.olingo.commons.core.edm.provider ==> seems to extend classes from "common-api" package "org.apache.olingo.commons.api.edm" which itself is confusing, because I would have expected them in "org.apache.olingo.commons.core.edm" package not "org.apache.olingo.commons.api.edm.provider". These are so similar to provider classes, and used by URL parsing routines, gets confused with provider classes. So, at a minimum change the package the name here.
server-api and server-core
--------------------------
I am not sure we really need interface abstraction really needed for those "uri" interfaces, but if somebody wants to develop a new server these will be useful.
odata-client-core
-------------------
org.apache.olingo.client.core.edm.xml ==> This one always gets me. These are exactly same as the provider classes above, except that they have JSON deserialization code. I am not sure what is being provided by "jackson" library here, olingo practically provides each step of the parsing, its not like JAXB where the XML object created for based on data structure. So, for seem these classes seem verbose. Two things we can do. Either make them package private so that only framework classes have visibility for parsing the return response for client. Or move all the Deserializer code into single utility class and get rid of all classes (not sure this possible), otherwise do not depend on json library (jackson) based automatic parsing, as I mentioned above this is providing little or no automation in creation object. Also, there is no $metadata payload coming in JSON yet, what are these used for?
Thank you.
Ramesh..
----- Original Message -----
> Hi Ramesh,
>
> naming is incredibly important in my opinion and I am always thankful for any
> kind of feedback on that.
>
> I had the same thoughts when creating the API and Provider classes. My
> thought there was to have the edm provider beans without EDM prefix and the
> EDM API with the prefix. I did not see that data beans at that time so there
> is a Property in the edm provider and a property in the data package.
>
> My first thought would be to prefix all Provider classes with "Prov" or
> something like that but I did not have the time for that.
>
> If you have any other naming ideas please let me know.
>
> Best Regards,
> Christian
>
> -----Original Message-----
> From: Ramesh Reddy [mailto:rareddy@redhat.com]
> Sent: Donnerstag, 16. April 2015 21:55
> To: dev@olingo.apache.org
> Subject: Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
>
> scratch that, I see some other classes with Edm prefix already. Surely as you
> can see even after looking at the code for sometime I still get confused
> with similarly named classes. Renaming them to correctly represent their
> function is really going help understand for service developers IMO. I
> understand they are in different packages, but does not do whole lot of
> good.
>
> I am still learning -:)
>
> Ramesh..
>
> ----- Original Message -----
> > Hi,
> >
> > Since we are refactoring lot of common-api, common-core code, is it
> > possible
> > to rename classes in "org.apache.olingo.commons.api.edm.provider" package
> > in
> > the common-api module to prefix something like "Edm", so they are
> > EdmAction,
> > EdmActionImport etc? I always get confused which classes to select when
> > some
> > of the classes with same name show up from
> > "org.apache.olingo.commons.api.data" package. By adding this prefix, it
> > would be much more clear IMO.
> >
> > what do you guys think?
> >
> > Ramesh..
> > http://teiid.org
> >
> >
>
RE: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
Posted by "Amend, Christian" <ch...@sap.com>.
Hi Ramesh,
naming is incredibly important in my opinion and I am always thankful for any kind of feedback on that.
I had the same thoughts when creating the API and Provider classes. My thought there was to have the edm provider beans without EDM prefix and the EDM API with the prefix. I did not see that data beans at that time so there is a Property in the edm provider and a property in the data package.
My first thought would be to prefix all Provider classes with "Prov" or something like that but I did not have the time for that.
If you have any other naming ideas please let me know.
Best Regards,
Christian
-----Original Message-----
From: Ramesh Reddy [mailto:rareddy@redhat.com]
Sent: Donnerstag, 16. April 2015 21:55
To: dev@olingo.apache.org
Subject: Re: Renaming classes in "org.apache.olingo.commons.api.edm.provider"
scratch that, I see some other classes with Edm prefix already. Surely as you can see even after looking at the code for sometime I still get confused with similarly named classes. Renaming them to correctly represent their function is really going help understand for service developers IMO. I understand they are in different packages, but does not do whole lot of good.
I am still learning -:)
Ramesh..
----- Original Message -----
> Hi,
>
> Since we are refactoring lot of common-api, common-core code, is it possible
> to rename classes in "org.apache.olingo.commons.api.edm.provider" package in
> the common-api module to prefix something like "Edm", so they are EdmAction,
> EdmActionImport etc? I always get confused which classes to select when some
> of the classes with same name show up from
> "org.apache.olingo.commons.api.data" package. By adding this prefix, it
> would be much more clear IMO.
>
> what do you guys think?
>
> Ramesh..
> http://teiid.org
>
>
Re: Renaming classes in
"org.apache.olingo.commons.api.edm.provider"
Posted by Ramesh Reddy <ra...@redhat.com>.
scratch that, I see some other classes with Edm prefix already. Surely as you can see even after looking at the code for sometime I still get confused with similarly named classes. Renaming them to correctly represent their function is really going help understand for service developers IMO. I understand they are in different packages, but does not do whole lot of good.
I am still learning -:)
Ramesh..
----- Original Message -----
> Hi,
>
> Since we are refactoring lot of common-api, common-core code, is it possible
> to rename classes in "org.apache.olingo.commons.api.edm.provider" package in
> the common-api module to prefix something like "Edm", so they are EdmAction,
> EdmActionImport etc? I always get confused which classes to select when some
> of the classes with same name show up from
> "org.apache.olingo.commons.api.data" package. By adding this prefix, it
> would be much more clear IMO.
>
> what do you guys think?
>
> Ramesh..
> http://teiid.org
>
>