You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@devicemap.apache.org by Reza <re...@yahoo.com.INVALID> on 2014/07/08 22:04:55 UTC
Re: moving XMLParser.java to loader.impl
So why are you singling out XMLParser.java and not any of the other public classes in the project? If we need to follow Law of Demeter, then we need to put all classes into a single package and make them private. No?
Also, your commit message for the move is:
"DMAP-38: Integrate W3C Simple DDR library
Task-Url: https://issues.apache.org/jira/browse/DMAP-38"
You did not address DMAP-38 in moving the XMLParser to the loader.impl package. So this is a completely invalid commit message. If you are going to commit a change, please use a proper commit message.
If commits like this continue, then we will have to put commits thru a code review process. Lets try and not take this step :)
________________________________
From: Werner Keil <we...@gmail.com>
To: "devicemap-dev@incubator.apache.org" <de...@incubator.apache.org>; Reza <re...@yahoo.com>
Sent: Tuesday, July 8, 2014 3:53 PM
Subject: Re: DeviceMap data and java client 1.0.0 release review ready
According to "Demeter's Law" (http://en.wikipedia.org/wiki/Law_of_Demeter) as few as possible public APIs should be exposed. Putting classes into different packages makes is hard under Java to accomplish that. Only let's say OSGi enabled projects (something we could add if using them e.g. in a container like Felix/Caraf is of interest) manage to declare some packages as "internal" by simply not exposing them in the OSGi "bundle".
If a parser is already or very soon meant to be used in the signatures of public methods, then why not offer them a separate packge. Otherwise we could wait and leave it with "impl".
On Tue, Jul 8, 2014 at 9:39 PM, Reza <re...@yahoo.com.invalid> wrote:
Technically 90% of the classes in the project are details of the implementation and are not exposed, does that mean that all those classes need to go into the same package? :)
>
>
>The packages are there for better organization and separation of work and interests. Parsers go into a parsers package, not the loader implementation package. At least, thats the way I have always done it.
>
>
>>> module name
>
>
>As for the svn project name, sure.
>
>
>
>
>
>
>________________________________
>
>From: Werner Keil <we...@gmail.com>
>To: "devicemap-dev@incubator.apache.org" <de...@incubator.apache.org>; Reza <re...@yahoo.com>
>Sent: Tuesday, July 8, 2014 3:27 PM
>
>Subject: Re: DeviceMap data and java client 1.0.0 release review ready
>
>
>
>It so far is a detail of the implementation that's not exposed. Only a private method refers to it. How would that change?
>
>
>
>
>On Tue, Jul 8, 2014 at 9:15 PM, Reza <re...@yahoo.com.invalid> wrote:
>
>I saw it. You had moved it into the loader.impl package. It doesnt belong there because its not a loader implementation. It goes into the loader.parser package since its a parser... When we add more parsers, they will go into that package as well.
>>
>>
>>
>>________________________________
>> From: Werner Keil <we...@gmail.com>
>>To: "devicemap-dev@incubator.apache.org" <de...@incubator.apache.org>; Reza <re...@yahoo.com>
>>Sent: Tuesday, July 8, 2014 3:11 PM
>>Subject: Re: DeviceMap data and java client 1.0.0 release review ready
>>
>>
>>
>>OK, I did the refactoring of the parser, you'll noticed it during the commit/update.
>>
>>
>>Actually it seems you missed it Please try to update before the next commit, so a tagged version no longer exposes a class that's not used outside...
>>
>>
>>
>>
>>
>>On Tue, Jul 8, 2014 at 9:03 PM, Reza <re...@yahoo.com.invalid> wrote:
>>
>>I have updated the devicemap data and java client release:
>>>
>>>http://www.rezsoft.org/devicemap/
>>>
>>>
>>>Changes:
>>>
>>>-fixed the LICENSE, NOTICE, DISCLAIMER. I also added these to individual projects to simplify releasing.
>>>-added a README
>>>-removed all the @author comments
>>>-changed version from 1.0 to 1.0.0
>>>-I changed the java client artifact id from devicemap-java to devicemap-client
>>>-I moved some classes around in the loader package so they are better organized
>>>
>>>If all looks good, I will create the tag for this release.
>>>
>>
>>
>>
>
>
>
Re: moving XMLParser.java to loader.impl
Posted by Werner Keil <we...@gmail.com>.
Went into wrong Mylyn context, sorry, I am just fixing the problem of the
JAR in a proper way.
On Tue, Jul 8, 2014 at 10:04 PM, Reza <re...@yahoo.com.invalid>
wrote:
> So why are you singling out XMLParser.java and not any of the other public
> classes in the project? If we need to follow Law of Demeter, then we need
> to put all classes into a single package and make them private. No?
>
> Also, your commit message for the move is:
>
> "DMAP-38: Integrate W3C Simple DDR library
>
> Task-Url: https://issues.apache.org/jira/browse/DMAP-38"
>
> You did not address DMAP-38 in moving the XMLParser to the loader.impl
> package. So this is a completely invalid commit message. If you are going
> to commit a change, please use a proper commit message.
>
> If commits like this continue, then we will have to put commits thru a
> code review process. Lets try and not take this step :)
>
>
> ________________________________
> From: Werner Keil <we...@gmail.com>
> To: "devicemap-dev@incubator.apache.org" <
> devicemap-dev@incubator.apache.org>; Reza <re...@yahoo.com>
> Sent: Tuesday, July 8, 2014 3:53 PM
> Subject: Re: DeviceMap data and java client 1.0.0 release review ready
>
>
>
> According to "Demeter's Law" (http://en.wikipedia.org/wiki/Law_of_Demeter)
> as few as possible public APIs should be exposed. Putting classes into
> different packages makes is hard under Java to accomplish that. Only let's
> say OSGi enabled projects (something we could add if using them e.g. in a
> container like Felix/Caraf is of interest) manage to declare some packages
> as "internal" by simply not exposing them in the OSGi "bundle".
>
> If a parser is already or very soon meant to be used in the signatures of
> public methods, then why not offer them a separate packge. Otherwise we
> could wait and leave it with "impl".
>
>
> On Tue, Jul 8, 2014 at 9:39 PM, Reza <re...@yahoo.com.invalid>
> wrote:
>
> Technically 90% of the classes in the project are details of the
> implementation and are not exposed, does that mean that all those classes
> need to go into the same package? :)
> >
> >
> >The packages are there for better organization and separation of work and
> interests. Parsers go into a parsers package, not the loader implementation
> package. At least, thats the way I have always done it.
> >
> >
> >>> module name
> >
> >
> >As for the svn project name, sure.
> >
> >
> >
> >
> >
> >
> >________________________________
> >
> >From: Werner Keil <we...@gmail.com>
> >To: "devicemap-dev@incubator.apache.org" <
> devicemap-dev@incubator.apache.org>; Reza <re...@yahoo.com>
> >Sent: Tuesday, July 8, 2014 3:27 PM
> >
> >Subject: Re: DeviceMap data and java client 1.0.0 release review ready
> >
> >
> >
> >It so far is a detail of the implementation that's not exposed. Only a
> private method refers to it. How would that change?
> >
> >
> >
> >
> >On Tue, Jul 8, 2014 at 9:15 PM, Reza <re...@yahoo.com.invalid>
> wrote:
> >
> >I saw it. You had moved it into the loader.impl package. It doesnt belong
> there because its not a loader implementation. It goes into the
> loader.parser package since its a parser... When we add more parsers, they
> will go into that package as well.
> >>
> >>
> >>
> >>________________________________
> >> From: Werner Keil <we...@gmail.com>
> >>To: "devicemap-dev@incubator.apache.org" <
> devicemap-dev@incubator.apache.org>; Reza <re...@yahoo.com>
> >>Sent: Tuesday, July 8, 2014 3:11 PM
> >>Subject: Re: DeviceMap data and java client 1.0.0 release review ready
> >>
> >>
> >>
> >>OK, I did the refactoring of the parser, you'll noticed it during the
> commit/update.
> >>
> >>
> >>Actually it seems you missed it Please try to update before the next
> commit, so a tagged version no longer exposes a class that's not used
> outside...
> >>
> >>
> >>
> >>
> >>
> >>On Tue, Jul 8, 2014 at 9:03 PM, Reza <re...@yahoo.com.invalid>
> wrote:
> >>
> >>I have updated the devicemap data and java client release:
> >>>
> >>>http://www.rezsoft.org/devicemap/
> >>>
> >>>
> >>>Changes:
> >>>
> >>>-fixed the LICENSE, NOTICE, DISCLAIMER. I also added these to
> individual projects to simplify releasing.
> >>>-added a README
> >>>-removed all the @author comments
> >>>-changed version from 1.0 to 1.0.0
> >>>-I changed the java client artifact id from devicemap-java to
> devicemap-client
> >>>-I moved some classes around in the loader package so they are better
> organized
> >>>
> >>>If all looks good, I will create the tag for this release.
> >>>
> >>
> >>
> >>
> >
> >
> >