On Mon, Apr 11, 2011 at 9:23 AM, Matt Benson <gudnabr...@gmail.com> wrote:
> On Sun, Apr 10, 2011 at 10:51 PM, Matt Benson <gudnabr...@gmail.com> wrote:
>> On Sun, Apr 10, 2011 at 7:17 PM, Henri Yandell <flame...@gmail.com> wrote:
>>> On Sun, Apr 10, 2011 at 1:43 PM, Phil Steitz <phil.ste...@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.
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-unsubscr...@commons.apache.org
>>> For additional commands, e-mail: dev-h...@commons.apache.org
>>>
>>>
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
