commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [text] Adapt the Log4j 2 Interpolator to [text]
Date Sun, 11 Feb 2018 21:46:46 GMT
On Sun, Feb 11, 2018 at 12:05 PM, Pascal Schumacher <
pascalschumacher@gmx.net> wrote:

> Am 11.02.2018 um 19:24 schrieb Gary Gregory:
>
>> I'd like a code review and then a release of 1.3. Right now we only depend
>> on java.base and Commons Lang, so let's keep it that way for 1.3 I think.
>>
> My comments:
>

Thank you for the code review!


> - Given "TEXT-80: StrLookup API confusing generic type parameter" I think
> we should deprecate the old StrLookup class and mark it for removal in
> commons-text 2.0.
>
+1 and will do.


> - DateStringLookup: should we use FastDateFormat?
>

I overlooked that one, I'll look into it.


> - AbstractStringLookup: empty class, I would therefore remove it.
>

I like to have it around for now, it is package private anyway. We can
remove it before 1.3 if it stays empty. At one point I have an
isEmpty(String) method in there before I realized that Commons Text depends
on Commons Lang which provides the same service in
StringUtils.isEmpty(String).


> - StringLookupFactory: should this be a static factory, to make it easier
> to use?


I am leaving room for configuring these things later so I feel that doing
.INSTANCE is a small price to pay but I hear you.

Gary


>
>
> (I almost added Log4j's JNDI lookup but I know that will not work on
>> Android so I'd like to leave stuff like that for later, maybe in a
>> different module.)
>>
> +1 for leaving it out
>
> -Pascal
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message