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 Mon, 12 Feb 2018 17:52:58 GMT
On Sun, Feb 11, 2018 at 8:12 PM, Chas Honton <chas@honton.org> wrote:

> Definitely create interfaces.
>

I agree 100% and will proceed. I thought about it overnight and it does not
make sense to leave a mix of abstract classes and interfaces in
StrSubstitutor.

Gary


>
> Chas
>
> > On Feb 11, 2018, at 1:57 PM, Gary Gregory <garydgregory@gmail.com>
> wrote:
> >
> > On Sun, Feb 11, 2018 at 2:46 PM, Gary Gregory <garydgregory@gmail.com>
> > wrote:
> >
> >>
> >>
> >> 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.
> >>
> >
> > Oh, and consider this alternative:
> >
> > - Create StringLookup (already there) and StringSubstitutor
> > - Deprecate StrLookup and StrSubstitutor and leave as is from 1.2
> > - ALSO deprecate StrMatcher which is a class and introduce a
> StringMatcher
> > _interface_.
> >
> > The idea here is to avoid having to do this a second time later. Right
> now
> > StrLookup and StrMatcher are classes, there are no interfaces. The
> question
> > is: Should we just redo the whole thing based on interfaces now. As of
> now
> > in master, we have a 50/50 situation with StrSubstituor supporting both
> > StrLookup and StringLookup AND using StrMatcher (a class, not an
> interface).
> >
> > Thoughts?
> >
> > Gary
> >
> >
> >> 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
> >>>
> >>>
> >>
>
> ---------------------------------------------------------------------
> 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