You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Pascal Schumacher <pa...@gmx.net> on 2016/12/18 18:06:19 UTC

[TEXT] Remove Commons Lang Dependency?

Hello everybody,

commons-text currently depends on commons-lang. This dependency is 
included into the jar via the maven shade plugin.

Only two methods from lang are really needed:

StringUtils#containsAny

StringUtils#containsNone

As dependencies should be minimized I'm not sure if adding a dependency 
for two methods (less than 60 lines of code) is a good idea.

What do you think?

-Pascal

P.S.: I have created a pull request: 
https://github.com/apache/commons-text/pull/18/files to demonstrate the 
changes necessary for the removal.







---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by sebb <se...@gmail.com>.
On 18 December 2016 at 22:02, Jörg Schaible <jo...@gmx.de> wrote:
> Pascal Schumacher wrote:
>
>> Hello everybody,
>>
>> commons-text currently depends on commons-lang. This dependency is
>> included into the jar via the maven shade plugin.
>>
>> Only two methods from lang are really needed:
>>
>> StringUtils#containsAny
>>
>> StringUtils#containsNone
>>
>> As dependencies should be minimized I'm not sure if adding a dependency
>> for two methods (less than 60 lines of code) is a good idea.
>>
>> What do you think?
>
> Actually I don't understand why do you want to remove it. It was made
> dependent on purpose. The shade plugin - if properly configured - will only
> include StringUtils as a private package in commons-text - so this
> dependency exists only at build time. We had a bad history of copied code in
> commons, because the original code got bug fixes and the copies were never
> updated later on.

In which case what needs to be done is to document this decision -
e.g. in the pom - so future maintainers don't have to go through this
again.

> Cheers,
> Jörg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by Jörg Schaible <jo...@gmx.de>.
Pascal Schumacher wrote:

> Am 19.12.2016 um 11:25 schrieb Pascal Schumacher:
>> Am 18.12.2016 um 23:02 schrieb J�rg Schaible:
>>> Actually I don't understand why do you want to remove it. It was made
>>> dependent on purpose. The shade plugin - if properly configured -
>>> will only
>>> include StringUtils as a private package in commons-text - so this
>>> dependency exists only at build time. We had a bad history of copied
>>> code in
>>> commons, because the original code got bug fixes and the copies were
>>> never
>>> updated later on.
>> Well the minimizing of jar by shade does not really work, maybe the
>> configuration is wrong? Even if commons-lang usage is reduced to just
>> StringUtils (https://github.com/apache/commons-text/pull/19/files) the
>> shade plugin still includes a lot of classes from lang (145 KB).
> 
> Sorry, I have to correct myself. The minimizing does work, StringUtils
> has (transitive) dependencies on half of lang.

OK, *that's* unfortunate. :-/

Cheers,
J�rg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by Pascal Schumacher <pa...@gmx.net>.
Am 19.12.2016 um 11:25 schrieb Pascal Schumacher:
> Am 18.12.2016 um 23:02 schrieb J�rg Schaible:
>> Actually I don't understand why do you want to remove it. It was made
>> dependent on purpose. The shade plugin - if properly configured - 
>> will only
>> include StringUtils as a private package in commons-text - so this
>> dependency exists only at build time. We had a bad history of copied 
>> code in
>> commons, because the original code got bug fixes and the copies were 
>> never
>> updated later on.
> Well the minimizing of jar by shade does not really work, maybe the 
> configuration is wrong? Even if commons-lang usage is reduced to just 
> StringUtils (https://github.com/apache/commons-text/pull/19/files) the 
> shade plugin still includes a lot of classes from lang (145 KB).

Sorry, I have to correct myself. The minimizing does work, StringUtils 
has (transitive) dependencies on half of lang.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by Pascal Schumacher <pa...@gmx.net>.
Am 18.12.2016 um 23:02 schrieb J�rg Schaible:
> Actually I don't understand why do you want to remove it. It was made
> dependent on purpose. The shade plugin - if properly configured - will only
> include StringUtils as a private package in commons-text - so this
> dependency exists only at build time. We had a bad history of copied code in
> commons, because the original code got bug fixes and the copies were never
> updated later on.
Well the minimizing of jar by shade does not really work, maybe the 
configuration is wrong? Even if commons-lang usage is reduced to just 
StringUtils (https://github.com/apache/commons-text/pull/19/files) the 
shade plugin still includes a lot of classes from lang (145 KB).

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by Jörg Schaible <jo...@gmx.de>.
Pascal Schumacher wrote:

> Hello everybody,
> 
> commons-text currently depends on commons-lang. This dependency is
> included into the jar via the maven shade plugin.
> 
> Only two methods from lang are really needed:
> 
> StringUtils#containsAny
> 
> StringUtils#containsNone
> 
> As dependencies should be minimized I'm not sure if adding a dependency
> for two methods (less than 60 lines of code) is a good idea.
> 
> What do you think?

Actually I don't understand why do you want to remove it. It was made 
dependent on purpose. The shade plugin - if properly configured - will only 
include StringUtils as a private package in commons-text - so this 
dependency exists only at build time. We had a bad history of copied code in 
commons, because the original code got bug fixes and the copies were never 
updated later on.

Cheers,
J�rg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by Rob Tompkins <ch...@gmail.com>.

> On Dec 18, 2016, at 5:08 PM, Bruno P. Kinoshita <br...@yahoo.com.br.INVALID> wrote:
> 
> Hi Pascal,
> 
> I suspect we may need a few other methods in the future, but for now
> I agree we are not using much of [lang] in [text].
> 
> If you would like to release 1.0 without the [lang] dependency, and then
> we review it later whether we include it or not, I'd be fine with that.
> Though I'd be happy with a 1.0 release with shaded methods too. So I
> believe my vote would be a +0 :)
> 

I'm a +0 here as well. Feels like fewer dependencies is a better practice, but I'm ok with including now or down the road. 

-Rob

> Hope that helps
> Bruno
> 
> 
> 
> 
> ----- Original Message -----
>> From: Pascal Schumacher <pa...@gmx.net>
>> To: Commons Developers List <de...@commons.apache.org>
>> Sent: Monday, 19 December 2016 7:06 AM
>> Subject: [TEXT] Remove Commons Lang Dependency?
>> 
>> Hello everybody,
>> 
>> commons-text currently depends on commons-lang. This dependency is 
>> included into the jar via the maven shade plugin.
>> 
>> Only two methods from lang are really needed:
>> 
>> StringUtils#containsAny
>> 
>> StringUtils#containsNone
>> 
>> As dependencies should be minimized I'm not sure if adding a dependency 
>> for two methods (less than 60 lines of code) is a good idea.
>> 
>> What do you think?
>> 
>> -Pascal
>> 
>> P.S.: I have created a pull request: 
>> https://github.com/apache/commons-text/pull/18/files to demonstrate the 
>> changes necessary for the removal.
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by "Bruno P. Kinoshita" <br...@yahoo.com.br.INVALID>.
Hi Pascal,

I suspect we may need a few other methods in the future, but for now
I agree we are not using much of [lang] in [text].

If you would like to release 1.0 without the [lang] dependency, and then
we review it later whether we include it or not, I'd be fine with that.
Though I'd be happy with a 1.0 release with shaded methods too. So I
believe my vote would be a +0 :)

Hope that helps
Bruno




----- Original Message -----
> From: Pascal Schumacher <pa...@gmx.net>
> To: Commons Developers List <de...@commons.apache.org>
> Sent: Monday, 19 December 2016 7:06 AM
> Subject: [TEXT] Remove Commons Lang Dependency?
> 
> Hello everybody,
> 
> commons-text currently depends on commons-lang. This dependency is 
> included into the jar via the maven shade plugin.
> 
> Only two methods from lang are really needed:
> 
> StringUtils#containsAny
> 
> StringUtils#containsNone
> 
> As dependencies should be minimized I'm not sure if adding a dependency 
> for two methods (less than 60 lines of code) is a good idea.
> 
> What do you think?
> 
> -Pascal
> 
> P.S.: I have created a pull request: 
> https://github.com/apache/commons-text/pull/18/files to demonstrate the 
> changes necessary for the removal.
> 
> 
> 
> 
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by sebb <se...@gmail.com>.
Looks OK.
However it might be confusing to have the same methods in both jars,
so at least the class needs to be renamed.

Probably best to make the methods internal.
Otherwise people might expect other related methods to be supported.
This would end up with duplication of LANG methods.

If it is later decided the TEXT should support more of such methods
they can be made external at that point.

On 18 December 2016 at 18:10, Matt Sicker <bo...@gmail.com> wrote:
> That looks good to me. We do similarly in Log4j. Commons classes tend to be
> written in such a way that you can copy/paste only what you need if you're
> trying to prevent required dependencies.
>
> On 18 December 2016 at 12:06, Pascal Schumacher <pa...@gmx.net>
> wrote:
>
>> Hello everybody,
>>
>> commons-text currently depends on commons-lang. This dependency is
>> included into the jar via the maven shade plugin.
>>
>> Only two methods from lang are really needed:
>>
>> StringUtils#containsAny
>>
>> StringUtils#containsNone
>>
>> As dependencies should be minimized I'm not sure if adding a dependency
>> for two methods (less than 60 lines of code) is a good idea.
>>
>> What do you think?
>>
>> -Pascal
>>
>> P.S.: I have created a pull request: https://github.com/apache/comm
>> ons-text/pull/18/files to demonstrate the changes necessary for the
>> removal.
>>
>>
>>
>>
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: [TEXT] Remove Commons Lang Dependency?

Posted by Matt Sicker <bo...@gmail.com>.
That looks good to me. We do similarly in Log4j. Commons classes tend to be
written in such a way that you can copy/paste only what you need if you're
trying to prevent required dependencies.

On 18 December 2016 at 12:06, Pascal Schumacher <pa...@gmx.net>
wrote:

> Hello everybody,
>
> commons-text currently depends on commons-lang. This dependency is
> included into the jar via the maven shade plugin.
>
> Only two methods from lang are really needed:
>
> StringUtils#containsAny
>
> StringUtils#containsNone
>
> As dependencies should be minimized I'm not sure if adding a dependency
> for two methods (less than 60 lines of code) is a good idea.
>
> What do you think?
>
> -Pascal
>
> P.S.: I have created a pull request: https://github.com/apache/comm
> ons-text/pull/18/files to demonstrate the changes necessary for the
> removal.
>
>
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
Matt Sicker <bo...@gmail.com>