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.
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
