harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Geir Magnusson Jr." <g...@pobox.com>
Subject Re: [classlib][awt] Revision #465514 broke image decoders.
Date Tue, 24 Oct 2006 20:52:28 GMT


Oleg Khaschansky wrote:
>> That's the start of a unit test, are you going to finish it? ;-)
> Well, I'll look into this. We need to put a couple of images for this
> kind of test somewhere...

How about a picture of you? :)

> 
>> But not a lot compared to the number of cleanup changes Nathan has been
>> busy making!
> Right, I am sorry. This cleanup is a great work. I suggested to roll
> back because I thought that there could be other places with similar
> issues that could have been missed. AWT has a lot of native code, so
> refactorings should be done with this in mind. The test coverage for
> AWT is quite poor, in comparison with the undelying functionality,
> it's quite easy to introduce a new bug in it.
> 
> On 10/24/06, Mark Hindess <mark.hindess@googlemail.com> wrote:
>>
>> On 24 October 2006 at 18:25, "Oleg Khaschansky" 
>> <oleg.v.khaschansky@gmail.com> wrote:
>> > > > Where are the tests for these decoders? How did you determine that
>> > > > they no longer worked?
>> >
>> > Unfortunately, these classes are not covered with the unit tests.
>> >
>> > I was running a simple test application that did something like this:
>> > Toolkit.getDefaultToolkit().getImage("image.jpg");
>> > and if failed with a NPE.
>>
>> That's the start of a unit test, are you going to finish it? ;-)
>>
>> > > I removed the final modifiers
>> > At the first glance it seems like the problem doesn't appear any more.
>> >
>> > > There were only 3-4 other fields that were finalized. Your email
>> > > mentioned "a lot of invalid modifications"; what are the other 
>> issues,
>> > > specifically?
>> > Only final fields. No other issues. But 3-4 in 3 classes - it is 
>> alot for me
>> > :)
>>
>> But not a lot compared to the number of cleanup changes Nathan has been
>> busy making!
>>
>> Regards,
>>  Mark.
>>
>> > On 10/24/06, Nathan Beyer <nbeyer@gmail.com> wrote:
>> > > I removed the final modifiers; this only affected PngDecoder,
>> > > GifDecoder and JpegDecoder. I missed the comments in the fields of
>> > > JpegDecoder, that was my mistake.
>> > >
>> > > There were only 3-4 other fields that were finalized. Your email
>> > > mentioned "a lot of invalid modifications"; what are the other 
>> issues,
>> > > specifically?
>> > >
>> > > -Nathan
>> > >
>> > > On 10/24/06, Nathan Beyer <nbeyer@gmail.com> wrote:
>> > > > Where are the tests for these decoders? How did you determine that
>> > > > they no longer worked?
>> > > >
>> > > > I'll remove the final modifiers.
>> > > >
>> > > > -Nathan
>> > > >
>> > > > On 10/24/06, Oleg Khaschansky <oleg.v.khaschansky@gmail.com>
wrote:
>> > > > > Hi,
>> > > > >
>> > > > > Rev. 465514 introduced a lot of invalid modifications to the
>> > > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of

>> fields
>> > > > > modified or initialized from the native code only, but they were
>> > > > > redeclared as final, so the decoders doesn't work properly any

>> more.
>> > > > >
>> > > > > This revision has the following comment:
>> > > > >
>> > > > > Cleanup code
>> > > > > * Add if/else braces
>> > > > > * Add missing annotations
>> > > > > * Add type variables
>> > > > > * Use foreach loops
>> > > > > * etc
>> > > > >
>> > > > > I'd suggest to roll back this revision and redo the cleanup in

>> the
>> > > > > more accurate way.
>> > > > >
>> > > > > Thanks,
>> > > > >   Oleg
>> > > > >
>> > > >
>> > >
>> >
>>
>>
>>

Mime
View raw message