On Sun, Feb 11, 2018 at 12:05 PM, Pascal Schumacher < pascalschumac...@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-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
