commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Benson <gudnabr...@gmail.com>
Subject Re: [VOTE] [LANG] Release Commons Lang 3.0 (based on RC2)
Date Mon, 11 Apr 2011 15:14:06 GMT
On Mon, Apr 11, 2011 at 10:03 AM, Matt Benson <gudnabrsam@gmail.com> wrote:
> On Mon, Apr 11, 2011 at 9:23 AM, Matt Benson <gudnabrsam@gmail.com> wrote:
>> On Sun, Apr 10, 2011 at 10:51 PM, Matt Benson <gudnabrsam@gmail.com> wrote:
>>> On Sun, Apr 10, 2011 at 7:17 PM, Henri Yandell <flamefew@gmail.com> wrote:
>>>> On Sun, Apr 10, 2011 at 1:43 PM, Phil Steitz <phil.steitz@gmail.com>
wrote:
>>>>> -0
>>>>>
>>>>> Would like to see findbugs warnings sorted.  I have not retested
>>>>> after Luc's fixes, but I in addition to the ones he mentions, I am
>>>>> not sure I understand the last one (on the site in ~bayard) and
>>>>> whether or not it is in fact a bug.  I think findbugs is complaining
>>>>> because
>>>>>
>>>>> *public* ExtendedMessageFormat(String pattern, Locale locale,
>>>>> Map<String, ? *extends* FormatFactory> registry) {
>>>>> *    super*(DUMMY_PATTERN);
>>>>>    setLocale(locale);
>>>>> *    this*.registry = registry;
>>>>>    applyPattern(pattern);
>>>>> }
>>>>>
>>>>> if you look at the source for MessageFormat, the constructor above
>>>>> calls applyPattern. The overridden version in [lang] tests if
>>>>> registry is null and delegates back to the super version if registry
>>>>> is null. This is probably OK, but findbugs is likely complaining
>>>>> because registry ends up getting read before it is initialized.
>>>>
>>>> Fixes definitely welcome on this one. My take has been the gamble that
>>>> it's not a backwards compat issue to fix and therefore not a reason to
>>>> block on.
>>>>
>>>>> * It would be great to fix the too long lines causing checkstyle
>>>>> problems as well or just get rid of the line length check. I will
>>>>> shorten the offending lines if you are OK with that.
>>>>
>>>> I've scratched my head on these a few times. Making our lines shorter
>>>> simply to get rid of a 120 char check is bad. None of the lines are
>>>> over-long for a reason.
>>>>
>>>> Minor non-release blocking issue.
>>>>
>>>>> * Need to address Matt's point and make sure otherwise this is the
>>>>> horse we are going to ride from API standoint. I would say "speak
>>>>> now or forever hold you peace" and cut another RC with the changes.
>>>>
>>>> The one example (WordUtils.capitalize) is extremely minor; we end up
>>>> having to maintain a 1 line method that links directly to the other
>>>> one. Annoying; but not the end of the world to have deprecated until
>>>> 4.0.
>>>>
>>>> Matt - do you have any idea how much we need to do here?
>>>
>>> I have been through everything in the base package and a little more
>>> than half the classes in the .builder package.  The last time I
>>> scanned "what's left" this was the most obvious thing I felt I could
>>> help with, so I simply began eyeballing every class in the [lang]
>>> codebase and making the necessary changes, but have had only limited
>>> time to devote to the task.  I don't expect the newer code--a few
>>> packages' worth--to have much to do, so most likely the majority of
>>> changes are already there.  I haven't, however, made sure of it yet.
>>> I will keep plugging away as time permits...  :|
>>
>> On this note, ExceptionUtils#getCause(Throwable)/(Throwable, String[])
>> is a candidate for this, although these methods are deprecated.  The
>> only problem is that an empty String array is currently treated
>> differently than a null String[], so we can't merge the methods
>> without breaking a test case.  If it were up to me I would make the
>> change, break the test case, and document the difference, but as the
>> methods are deprecated anyway I know others might feel differently.
>>
>
> Next issue:  WordUtils.capitalize/uncapitalize/capitalizeFully/initials
> all should IMO have their (String) + (String, char...) variants
> merged, but the methods bearing the (String, char...) are currently
> defined to return the first argument if the second argument has a
> length of zero (though, to support the (String) signature, not if the
> array reference is null).  In order to make this change we would have
> to come to a consensus to make empty arrays behave as null ones.
>

Besides the ExceptionUtils and WordUtils cases I have noted for
discussion, I have completed my review of APIs to be merged based on
varargs usage.

Matt

> Matt
>
>> Matt
>>
>>>
>>> Matt
>>>
>>>>
>>>>> * One last nit - why did we decide to dump the Ant build. Version
>>>>> 2.6 seems to have a working Ant build. Why wouldn't the same build
>>>>> work for 3.0. If you are OK with this, I will try to get the Ant
>>>>> build restored.
>>>>
>>>> IIRC, because no one was maintaining it. I've dumped other Ant builds
>>>> in other components too over the last 4 years (along with maven1
>>>> builds). I'm generally -1 to the "there are many ways to build it"
>>>> approach. It takes the pain of dealing with one build system and
>>>> increases it to 3x the pain. [manage build1, manage build2 and then
>>>> ensure build1 and build2 stay in sync].
>>>>
>>>> What's the scope of the Ant build? Just to build and run the unit
>>>> tests, or more than that?
>>>>
>>>> Can you create a Ant script that does that based on the pom?
>>>>
>>>> Hen
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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
View raw message