Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)
On 07/04/2020 09:40, Emmanuel Bourg wrote: > Le 07/04/2020 à 10:12, Mark Thomas a écrit : > >> The dependencies are only embedded to create a single JAR to make it >> easier for users to use on the command line. If the code was re-used I'd >> expect the "standard" JAR to be used and leave the decision on how to >> handle the dependencies up to the integrator. > > Ok, I assumed the shaded jar was the main artifact. As long as it isn't > published to Maven Central I'm fine with removing the relocation. Working out how this could be formally released is still on the TODO list. I don't view the shaded JAR as the main artefact but I can see it being pushed to Maven Central at some point. If that does happen it would be along side the "standard" JAR which would have a POM with the correct dependencies. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)
Le 07/04/2020 à 10:12, Mark Thomas a écrit : > The dependencies are only embedded to create a single JAR to make it > easier for users to use on the command line. If the code was re-used I'd > expect the "standard" JAR to be used and leave the decision on how to > handle the dependencies up to the integrator. Ok, I assumed the shaded jar was the main artifact. As long as it isn't published to Maven Central I'm fine with removing the relocation. Emmanuel Bourg - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)
On 06/04/2020 17:45, Emmanuel Bourg wrote: > Le 06/04/2020 à 18:33, Mark Thomas a écrit : > >> OK. But that is still 7.2k of classes rather than 2.4k of classes for >> zero benefit. >> >> I'll withdraw my -1 because we are approaching the point where the >> differences aren't worth the time spent discussing them but I still >> don't like this change. > > I've removed the duplicated Apache license in the shaded jar, so the > difference is now near zero. > > I agree this change is trivial and doesn't bring significant changes. > Apache Commons was created to share and reuse common code used at the > ASF, I always feel a bit sad when the code produced there isn't reused. > > >> And the relocation of the dependencies? At the moment, I only see a >> downside (harder debugging). What is the benefit? > > Relocating embedded dependencies is a best practice to prevent classpath > conflicts. If the tools ends up being integrated in a bigger software it > could clash with another version of BCEL on the classpath. The dependencies are only embedded to create a single JAR to make it easier for users to use on the command line. If the code was re-used I'd expect the "standard" JAR to be used and leave the decision on how to handle the dependencies up to the integrator. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)
Le 06/04/2020 à 18:33, Mark Thomas a écrit : > OK. But that is still 7.2k of classes rather than 2.4k of classes for > zero benefit. > > I'll withdraw my -1 because we are approaching the point where the > differences aren't worth the time spent discussing them but I still > don't like this change. I've removed the duplicated Apache license in the shaded jar, so the difference is now near zero. I agree this change is trivial and doesn't bring significant changes. Apache Commons was created to share and reuse common code used at the ASF, I always feel a bit sad when the code produced there isn't reused. > And the relocation of the dependencies? At the moment, I only see a > downside (harder debugging). What is the benefit? Relocating embedded dependencies is a best practice to prevent classpath conflicts. If the tools ends up being integrated in a bigger software it could clash with another version of BCEL on the classpath. Emmanuel Bourg - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)
On 06/04/2020 17:18, Emmanuel Bourg wrote:
> Le 06/04/2020 à 17:50, Mark Thomas a écrit :
>
>>> from 5c96c0b Ignore the IntelliJ project files
>>> new 29ea189 Replaced NonClosing{In,Out}putStream with the equivalent
>>> classes from Commons IO
>>
>> -1. It adds 220k of bloat for no benefit.
>
> No the dependencies are shaded with the minimizeJar option enabled, only
> the classes used are kept in the final jar.
OK. But that is still 7.2k of classes rather than 2.4k of classes for
zero benefit.
I'll withdraw my -1 because we are approaching the point where the
differences aren't worth the time spent discussing them but I still
don't like this change.
>>> new 528ccfa Made the internal classes package private
>>
>> I am close to -1 on this change.
>>
>> I would rather these were left public at this stage in the development
>> of this tool to make it as easy as possible for folks to tinker with
>> this code, re-using the bits that work for them. Longer term I had the
>> possibility in mind that users might need to register custom Converters
>> so making that package private seems very out of place.
>>
>> I get that we can always relax visibility rules later but this change
>> looks premature to me.
>
> Ok sounds fair, I've reverted it.
And the relocation of the dependencies? At the moment, I only see a
downside (harder debugging). What is the benefit?
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)
Le 06/04/2020 à 17:50, Mark Thomas a écrit :
>> from 5c96c0b Ignore the IntelliJ project files
>> new 29ea189 Replaced NonClosing{In,Out}putStream with the equivalent
>> classes from Commons IO
>
> -1. It adds 220k of bloat for no benefit.
No the dependencies are shaded with the minimizeJar option enabled, only
the classes used are kept in the final jar.
>> new 528ccfa Made the internal classes package private
>
> I am close to -1 on this change.
>
> I would rather these were left public at this stage in the development
> of this tool to make it as easy as possible for folks to tinker with
> this code, re-using the bits that work for them. Longer term I had the
> possibility in mind that users might need to register custom Converters
> so making that package private seems very out of place.
>
> I get that we can always relax visibility rules later but this change
> looks premature to me.
Ok sounds fair, I've reverted it.
Emmanuel Bourg
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat-jakartaee-migration] branch master updated (5c96c0b -> 61ba095)
On 06/04/2020 14:34, ebo...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> ebourg pushed a change to branch master
> in repository
> https://gitbox.apache.org/repos/asf/tomcat-jakartaee-migration.git.
>
>
> from 5c96c0b Ignore the IntelliJ project files
> new 29ea189 Replaced NonClosing{In,Out}putStream with the equivalent
> classes from Commons IO
-1. It adds 220k of bloat for no benefit.
> new 528ccfa Made the internal classes package private
I am close to -1 on this change.
I would rather these were left public at this stage in the development
of this tool to make it as easy as possible for folks to tinker with
this code, re-using the bits that work for them. Longer term I had the
possibility in mind that users might need to register custom Converters
so making that package private seems very out of place.
I get that we can always relax visibility rules later but this change
looks premature to me.
> new 61ba095 Relocated the dependencies in the shaded jar
Again, I am close to -1 on this change.
Why? I don't see the need for this. It just makes debugging potentially
harder.
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
