[Discussion] Coding practice for location of internal package name

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[Discussion] Coding practice for location of internal package name

vmassol
Administrator
Hi devs,

On Matrix/IRC, I’ve posted the following:

"
* Guillaume Delhumeau: BTW your naming is strange for the internal package
* for ex: package org.xwiki.notifications.preferences.internal.email;
* normally we put internal just after the main package part
* ie.
* org.xwiki.notifications.internal.*
* and org.xwiki.notifications.* for public classes
* see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/#HPackagenames
* General rule is org.xwiki.(module name).internal.
* I see thomas has done the same “error" for org.xwiki.job.handler.internal and org.xwiki.job.handler.internal.question . So maybe there's something to discuss/change
* I guess we have a mix of both now so we should discuss it and adjust our rules if need be


So I think we don’t have all the same rules/understanding of the definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/#HPackagenames

I’d like to discuss with you to see what you prefer and adjust our rules so that it matches what we do in practice.

Any take in this?

Thanks
-Vincent
Reply | Threaded
Open this post in threaded view
|

Re: [Discussion] Coding practice for location of internal package name

Guillaume Delhumeau
When I need to write code that I don't want to be seen as public API, I put
it in the internal package (that's the definition).

Now, if I have a lot of code that I want to keep internal, I create
sub-packages under that "internal" package.

Because sometimes, it doesn't make any sense to create dozens of empty
packages just to have an "internal" package at the deeper level that
actually have code.

Thanks

2018-04-16 18:28 GMT+02:00 Vincent Massol <[hidden email]>:

> Hi devs,
>
> On Matrix/IRC, I’ve posted the following:
>
> "
> * Guillaume Delhumeau: BTW your naming is strange for the internal package
> * for ex: package org.xwiki.notifications.preferences.internal.email;
> * normally we put internal just after the main package part
> * ie.
> * org.xwiki.notifications.internal.*
> * and org.xwiki.notifications.* for public classes
> * see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
> JavaCodeStyle/#HPackagenames
> * General rule is org.xwiki.(module name).internal.
> * I see thomas has done the same “error" for
> org.xwiki.job.handler.internal and org.xwiki.job.handler.internal.question
> . So maybe there's something to discuss/change
> * I guess we have a mix of both now so we should discuss it and adjust our
> rules if need be
> “
>
> So I think we don’t have all the same rules/understanding of the
> definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
> JavaCodeStyle/#HPackagenames
>
> I’d like to discuss with you to see what you prefer and adjust our rules
> so that it matches what we do in practice.
>
> Any take in this?
>
> Thanks
> -Vincent




--
Guillaume Delhumeau ([hidden email])
Research & Development Engineer at XWiki SAS
Committer on the XWiki.org project
Reply | Threaded
Open this post in threaded view
|

Re: [Discussion] Coding practice for location of internal package name

Marius Dumitru Florea
In reply to this post by vmassol
I think I prefer having the internal package closer to the root (so that
it's easier to spot it) but I may have made the same "error" like Guillaume
and Thomas in the past.

On Mon, Apr 16, 2018 at 7:28 PM, Vincent Massol <[hidden email]> wrote:

> Hi devs,
>
> On Matrix/IRC, I’ve posted the following:
>
> "
> * Guillaume Delhumeau: BTW your naming is strange for the internal package
> * for ex: package org.xwiki.notifications.preferences.internal.email;
> * normally we put internal just after the main package part
> * ie.
> * org.xwiki.notifications.internal.*
> * and org.xwiki.notifications.* for public classes
> * see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
> JavaCodeStyle/#HPackagenames
> * General rule is org.xwiki.(module name).internal.
> * I see thomas has done the same “error" for
> org.xwiki.job.handler.internal and org.xwiki.job.handler.internal.question
> . So maybe there's something to discuss/change
> * I guess we have a mix of both now so we should discuss it and adjust our
> rules if need be
> “
>
> So I think we don’t have all the same rules/understanding of the
> definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
> JavaCodeStyle/#HPackagenames
>
> I’d like to discuss with you to see what you prefer and adjust our rules
> so that it matches what we do in practice.
>
> Any take in this?
>
> Thanks
> -Vincent
Reply | Threaded
Open this post in threaded view
|

Re: [Discussion] Coding practice for location of internal package name

caubin
Hi devs,

On 04/18/2018 10:45 AM, Marius Dumitru Florea wrote:
> I think I prefer having the internal package closer to the root (so that
> it's easier to spot it) but I may have made the same "error" like Guillaume
> and Thomas in the past.

IMO, the current rule is incomplete as we do not take into account
submodules (which are the root cause of this conversation).

In the example given by Vincent, the package
org.xwiki.notifications.preferences.internal is part of the module
xwiki-platform-notifications-preferences : if we consider that the name
of the module is notification-preferences, then it's logical to put the
internal subpackage here.

Note that this is the same idea for org.xwiki.job.handler.internal,
which is in the submodule xwiki-platform-job-handler.

We then have to choose either to accept this format or totally discard
submodules.

> On Mon, Apr 16, 2018 at 7:28 PM, Vincent Massol <[hidden email]> wrote:
>
>> Hi devs,
>>
>> On Matrix/IRC, I’ve posted the following:
>>
>> "
>> * Guillaume Delhumeau: BTW your naming is strange for the internal package
>> * for ex: package org.xwiki.notifications.preferences.internal.email;
>> * normally we put internal just after the main package part
>> * ie.
>> * org.xwiki.notifications.internal.*
>> * and org.xwiki.notifications.* for public classes
>> * see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
>> JavaCodeStyle/#HPackagenames
>> * General rule is org.xwiki.(module name).internal.
>> * I see thomas has done the same “error" for
>> org.xwiki.job.handler.internal and org.xwiki.job.handler.internal.question
>> . So maybe there's something to discuss/change
>> * I guess we have a mix of both now so we should discuss it and adjust our
>> rules if need be
>> “
>>
>> So I think we don’t have all the same rules/understanding of the
>> definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
>> JavaCodeStyle/#HPackagenames
>>
>> I’d like to discuss with you to see what you prefer and adjust our rules
>> so that it matches what we do in practice.
>>
>> Any take in this?
>>
>> Thanks
>> -Vincent
Reply | Threaded
Open this post in threaded view
|

Re: [Discussion] Coding practice for location of internal package name

Thomas Mortagne
Administrator
On Wed, Apr 18, 2018 at 6:43 PM, Clément Aubin <[hidden email]> wrote:

> Hi devs,
>
> On 04/18/2018 10:45 AM, Marius Dumitru Florea wrote:
>> I think I prefer having the internal package closer to the root (so that
>> it's easier to spot it) but I may have made the same "error" like Guillaume
>> and Thomas in the past.
>
> IMO, the current rule is incomplete as we do not take into account
> submodules (which are the root cause of this conversation).
>
> In the example given by Vincent, the package
> org.xwiki.notifications.preferences.internal is part of the module
> xwiki-platform-notifications-preferences : if we consider that the name
> of the module is notification-preferences, then it's logical to put the
> internal subpackage here.
>
> Note that this is the same idea for org.xwiki.job.handler.internal,
> which is in the submodule xwiki-platform-job-handler.

Yep that's the idea.

The rule I usually apply IS "org.xwiki.(module name).internal." from
my point of view or to me more accurate the rule way I see it would be
better described as "everything in the module have the same package
prefix" so "org.xwiki.job.handler" in that example. The thing is you
interpret "module name" as "top level module name" which I don't
really agree with.
http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/#HPackagenames
is not really explicit about submodules in practice.

>
> We then have to choose either to accept this format or totally discard
> submodules.
>
>> On Mon, Apr 16, 2018 at 7:28 PM, Vincent Massol <[hidden email]> wrote:
>>
>>> Hi devs,
>>>
>>> On Matrix/IRC, I’ve posted the following:
>>>
>>> "
>>> * Guillaume Delhumeau: BTW your naming is strange for the internal package
>>> * for ex: package org.xwiki.notifications.preferences.internal.email;
>>> * normally we put internal just after the main package part
>>> * ie.
>>> * org.xwiki.notifications.internal.*
>>> * and org.xwiki.notifications.* for public classes
>>> * see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
>>> JavaCodeStyle/#HPackagenames
>>> * General rule is org.xwiki.(module name).internal.
>>> * I see thomas has done the same “error" for
>>> org.xwiki.job.handler.internal and org.xwiki.job.handler.internal.question
>>> . So maybe there's something to discuss/change
>>> * I guess we have a mix of both now so we should discuss it and adjust our
>>> rules if need be
>>> “
>>>
>>> So I think we don’t have all the same rules/understanding of the
>>> definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
>>> JavaCodeStyle/#HPackagenames
>>>
>>> I’d like to discuss with you to see what you prefer and adjust our rules
>>> so that it matches what we do in practice.
>>>
>>> Any take in this?
>>>
>>> Thanks
>>> -Vincent



--
Thomas Mortagne
Reply | Threaded
Open this post in threaded view
|

Re: [Discussion] Coding practice for location of internal package name

Eduard Moraru
+1 for the definition of "module" (in "module name") as general maven
module and not only top level modules.

i.e. a package name
"org.xwiki.module.submodule1.submodule2.submoduleN.*internal*.package1.package2.packageN"
corresponding to a maven artifact with ID
"org.xwiki.module.submodule1.submodule2.submoduleN".

Thanks,
Eduard

On Wed, Apr 18, 2018 at 7:59 PM, Thomas Mortagne <[hidden email]>
wrote:

> On Wed, Apr 18, 2018 at 6:43 PM, Clément Aubin <[hidden email]>
> wrote:
> > Hi devs,
> >
> > On 04/18/2018 10:45 AM, Marius Dumitru Florea wrote:
> >> I think I prefer having the internal package closer to the root (so that
> >> it's easier to spot it) but I may have made the same "error" like
> Guillaume
> >> and Thomas in the past.
> >
> > IMO, the current rule is incomplete as we do not take into account
> > submodules (which are the root cause of this conversation).
> >
> > In the example given by Vincent, the package
> > org.xwiki.notifications.preferences.internal is part of the module
> > xwiki-platform-notifications-preferences : if we consider that the name
> > of the module is notification-preferences, then it's logical to put the
> > internal subpackage here.
> >
> > Note that this is the same idea for org.xwiki.job.handler.internal,
> > which is in the submodule xwiki-platform-job-handler.
>
> Yep that's the idea.
>
> The rule I usually apply IS "org.xwiki.(module name).internal." from
> my point of view or to me more accurate the rule way I see it would be
> better described as "everything in the module have the same package
> prefix" so "org.xwiki.job.handler" in that example. The thing is you
> interpret "module name" as "top level module name" which I don't
> really agree with.
> http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
> JavaCodeStyle/#HPackagenames
> is not really explicit about submodules in practice.
>
> >
> > We then have to choose either to accept this format or totally discard
> > submodules.
> >
> >> On Mon, Apr 16, 2018 at 7:28 PM, Vincent Massol <[hidden email]>
> wrote:
> >>
> >>> Hi devs,
> >>>
> >>> On Matrix/IRC, I’ve posted the following:
> >>>
> >>> "
> >>> * Guillaume Delhumeau: BTW your naming is strange for the internal
> package
> >>> * for ex: package org.xwiki.notifications.preferences.internal.email;
> >>> * normally we put internal just after the main package part
> >>> * ie.
> >>> * org.xwiki.notifications.internal.*
> >>> * and org.xwiki.notifications.* for public classes
> >>> * see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
> >>> JavaCodeStyle/#HPackagenames
> >>> * General rule is org.xwiki.(module name).internal.
> >>> * I see thomas has done the same “error" for
> >>> org.xwiki.job.handler.internal and org.xwiki.job.handler.
> internal.question
> >>> . So maybe there's something to discuss/change
> >>> * I guess we have a mix of both now so we should discuss it and adjust
> our
> >>> rules if need be
> >>> “
> >>>
> >>> So I think we don’t have all the same rules/understanding of the
> >>> definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
> >>> JavaCodeStyle/#HPackagenames
> >>>
> >>> I’d like to discuss with you to see what you prefer and adjust our
> rules
> >>> so that it matches what we do in practice.
> >>>
> >>> Any take in this?
> >>>
> >>> Thanks
> >>> -Vincent
>
>
>
> --
> Thomas Mortagne
>
Reply | Threaded
Open this post in threaded view
|

Re: [Discussion] Coding practice for location of internal package name

Sergiu Dumitriu-3
+1 to what Eduard said.

On 04/20/2018 05:34 AM, Eduard Moraru wrote:

> +1 for the definition of "module" (in "module name") as general maven
> module and not only top level modules.
>
> i.e. a package name
> "org.xwiki.module.submodule1.submodule2.submoduleN.*internal*.package1.package2.packageN"
> corresponding to a maven artifact with ID
> "org.xwiki.module.submodule1.submodule2.submoduleN".
>
> Thanks,
> Eduard
>
> On Wed, Apr 18, 2018 at 7:59 PM, Thomas Mortagne <[hidden email]>
> wrote:
>
>> On Wed, Apr 18, 2018 at 6:43 PM, Clément Aubin <[hidden email]>
>> wrote:
>>> Hi devs,
>>>
>>> On 04/18/2018 10:45 AM, Marius Dumitru Florea wrote:
>>>> I think I prefer having the internal package closer to the root (so that
>>>> it's easier to spot it) but I may have made the same "error" like
>> Guillaume
>>>> and Thomas in the past.
>>>
>>> IMO, the current rule is incomplete as we do not take into account
>>> submodules (which are the root cause of this conversation).
>>>
>>> In the example given by Vincent, the package
>>> org.xwiki.notifications.preferences.internal is part of the module
>>> xwiki-platform-notifications-preferences : if we consider that the name
>>> of the module is notification-preferences, then it's logical to put the
>>> internal subpackage here.
>>>
>>> Note that this is the same idea for org.xwiki.job.handler.internal,
>>> which is in the submodule xwiki-platform-job-handler.
>>
>> Yep that's the idea.
>>
>> The rule I usually apply IS "org.xwiki.(module name).internal." from
>> my point of view or to me more accurate the rule way I see it would be
>> better described as "everything in the module have the same package
>> prefix" so "org.xwiki.job.handler" in that example. The thing is you
>> interpret "module name" as "top level module name" which I don't
>> really agree with.
>> http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
>> JavaCodeStyle/#HPackagenames
>> is not really explicit about submodules in practice.
>>
>>>
>>> We then have to choose either to accept this format or totally discard
>>> submodules.
>>>
>>>> On Mon, Apr 16, 2018 at 7:28 PM, Vincent Massol <[hidden email]>
>> wrote:
>>>>
>>>>> Hi devs,
>>>>>
>>>>> On Matrix/IRC, I’ve posted the following:
>>>>>
>>>>> "
>>>>> * Guillaume Delhumeau: BTW your naming is strange for the internal
>> package
>>>>> * for ex: package org.xwiki.notifications.preferences.internal.email;
>>>>> * normally we put internal just after the main package part
>>>>> * ie.
>>>>> * org.xwiki.notifications.internal.*
>>>>> * and org.xwiki.notifications.* for public classes
>>>>> * see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
>>>>> JavaCodeStyle/#HPackagenames
>>>>> * General rule is org.xwiki.(module name).internal.
>>>>> * I see thomas has done the same “error" for
>>>>> org.xwiki.job.handler.internal and org.xwiki.job.handler.
>> internal.question
>>>>> . So maybe there's something to discuss/change
>>>>> * I guess we have a mix of both now so we should discuss it and adjust
>> our
>>>>> rules if need be
>>>>> “
>>>>>
>>>>> So I think we don’t have all the same rules/understanding of the
>>>>> definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/
>>>>> JavaCodeStyle/#HPackagenames
>>>>>
>>>>> I’d like to discuss with you to see what you prefer and adjust our
>> rules
>>>>> so that it matches what we do in practice.
>>>>>
>>>>> Any take in this?
>>>>>
>>>>> Thanks
>>>>> -Vincent
>>
>>
>>
>> --
>> Thomas Mortagne
>>


--
Sergiu Dumitriu
http://purl.org/net/sergiu
Reply | Threaded
Open this post in threaded view
|

Re: [Discussion] Coding practice for location of internal package name

vmassol
Administrator
In reply to this post by vmassol
Hi devs,

To progress on this topic, I’m trying to summarize the options we have since it’s still not clear in my mind.

Option 1: Internal package after the 1st package
========

Class FQN example:
* org.xwiki.<package1>.internal.<package2>.<package3>.MyInternalClass

Pros:
* Easy to find all internal code for a given module (they’re all grouped together)
* Cons: ?

Devs:
* Vincent
* Marius

Option 2: Internal package at any level in the package hierarchy
========

Package examples:
* org.xwiki.<package1>.internal.<package2>.<package3>.MyInternalClass
* org.xwiki.<package1>.<package2>.internal.<package3>.MyInternalClass
* org.xwiki.<package1>.<package2>.<package3>.internal.MyInternalClass

Pros:
* ?

Cons:
* Internal code scattered in more packages.
* No consistency. Some devs will use org.xwiki.<package1>.internal.<package2>.<package3>.MyInternalClass when others will use org.xwiki.<package1>.<package2>.internal.<package3>.MyInternalClass and yet others will use org.xwiki.<package1>.<package2>.<package3>.internal.MyInternalClass

Devs:
* Thomas
* Edy
* Sergiu
* Guillaume

Conclusion
=========

@Thomas, Edy, Guillaume, Sergiu: could you confirm that you’re in favor of option 2? If so could you aslo explain the Pros of it and the Cons you see for option 1. Right now I see more pros for Option 1. But maybe I misunderstood what you’re proposing. Maybe you’re suggesting an option 3, based on the Maven module names?

Thanks
-Vincent


> On 16 Apr 2018, at 18:28, Vincent Massol <[hidden email]> wrote:
>
> Hi devs,
>
> On Matrix/IRC, I’ve posted the following:
>
> "
> * Guillaume Delhumeau: BTW your naming is strange for the internal package
> * for ex: package org.xwiki.notifications.preferences.internal.email;
> * normally we put internal just after the main package part
> * ie.
> * org.xwiki.notifications.internal.*
> * and org.xwiki.notifications.* for public classes
> * see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/#HPackagenames
> * General rule is org.xwiki.(module name).internal.
> * I see thomas has done the same “error" for org.xwiki.job.handler.internal and org.xwiki.job.handler.internal.question . So maybe there's something to discuss/change
> * I guess we have a mix of both now so we should discuss it and adjust our rules if need be
> “
>
> So I think we don’t have all the same rules/understanding of the definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/#HPackagenames
>
> I’d like to discuss with you to see what you prefer and adjust our rules so that it matches what we do in practice.
>
> Any take in this?
>
> Thanks
> -Vincent

Reply | Threaded
Open this post in threaded view
|

Re: [Discussion] Coding practice for location of internal package name

Thomas Mortagne
Administrator
On Tue, Dec 25, 2018 at 8:12 PM Vincent Massol <[hidden email]> wrote:

>
> Hi devs,
>
> To progress on this topic, I’m trying to summarize the options we have since it’s still not clear in my mind.
>
> Option 1: Internal package after the 1st package
> ========
>
> Class FQN example:
> * org.xwiki.<package1>.internal.<package2>.<package3>.MyInternalClass
>
> Pros:
> * Easy to find all internal code for a given module (they’re all grouped together)
> * Cons: ?
>
> Devs:
> * Vincent
> * Marius
>
> Option 2: Internal package at any level in the package hierarchy
> ========
>
> Package examples:
> * org.xwiki.<package1>.internal.<package2>.<package3>.MyInternalClass
> * org.xwiki.<package1>.<package2>.internal.<package3>.MyInternalClass
> * org.xwiki.<package1>.<package2>.<package3>.internal.MyInternalClass
>
> Pros:
> * ?
>
> Cons:
> * Internal code scattered in more packages.
> * No consistency. Some devs will use org.xwiki.<package1>.internal.<package2>.<package3>.MyInternalClass when others will use org.xwiki.<package1>.<package2>.internal.<package3>.MyInternalClass and yet others will use org.xwiki.<package1>.<package2>.<package3>.internal.MyInternalClass
>
> Devs:
> * Thomas
> * Edy
> * Sergiu
> * Guillaume

I did not suggested to randomly choose the package based on the
programmer mood which seems to be what you understood. What I proposed
is pretty much the same as Option1 but the difference is that where
you define what's between "org.xwiki" and "internal" based on the top
level module only I would prefer to use the whole hierarchy. One of
the goals being to have all the classes of the same module located in
the same base package instead of have two very different hierarchies.

So for example in your version, the module xwiki-rendering-macro-box
looks like this (what we have currently):

org.xwiki.rendering.internal.macro.box.DefaultBoxMacro
org.xwiki.rendering.macro.box.AbstractBoxMacro
org.xwiki.rendering.macro.box.BoxMacroParameters

and mine would be more like this:

org.xwiki.rendering.macro.box.AbstractBoxMacro
org.xwiki.rendering.macro.box.BoxMacroParameters
org.xwiki.rendering.macro.box.internal.DefaultBoxMacro

With this version everything related to the box macro is located under
org.xwiki.rendering.macro.box.**. In short everything which is part of
a specific domain or subdomain important enough to deserve its own sub
hierarchy has a dedicated base package for public package right now
and there is no reason for related internal classes end up in a
different hierarchy.

Of course the idea is not to include in the hierarchy modules
separated for very technical reasons (xwiki-platform-core, *-api,
*-default, etc.) like we currently do for public packages.

>
> Conclusion
> =========
>
> @Thomas, Edy, Guillaume, Sergiu: could you confirm that you’re in favor of option 2? If so could you aslo explain the Pros of it and the Cons you see for option 1. Right now I see more pros for Option 1. But maybe I misunderstood what you’re proposing. Maybe you’re suggesting an option 3, based on the Maven module names?
>
> Thanks
> -Vincent
>
>
> > On 16 Apr 2018, at 18:28, Vincent Massol <[hidden email]> wrote:
> >
> > Hi devs,
> >
> > On Matrix/IRC, I’ve posted the following:
> >
> > "
> > * Guillaume Delhumeau: BTW your naming is strange for the internal package
> > * for ex: package org.xwiki.notifications.preferences.internal.email;
> > * normally we put internal just after the main package part
> > * ie.
> > * org.xwiki.notifications.internal.*
> > * and org.xwiki.notifications.* for public classes
> > * see http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/#HPackagenames
> > * General rule is org.xwiki.(module name).internal.
> > * I see thomas has done the same “error" for org.xwiki.job.handler.internal and org.xwiki.job.handler.internal.question . So maybe there's something to discuss/change
> > * I guess we have a mix of both now so we should discuss it and adjust our rules if need be
> > “
> >
> > So I think we don’t have all the same rules/understanding of the definition at http://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/#HPackagenames
> >
> > I’d like to discuss with you to see what you prefer and adjust our rules so that it matches what we do in practice.
> >
> > Any take in this?
> >
> > Thanks
> > -Vincent
>


--
Thomas Mortagne