Mailing-List: contact harmony-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: harmony-dev@incubator.apache.org
Received-SPF: pass (herse.apache.org: domain of oleg.v.khaschansky@gmail.com
 designates 66.249.92.170 as permitted sender)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=gmail.com;
        h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references;
        b=RoO4NlrQ2RqCxpA6nPW7M3nMLN8Okl1KUFoJ4z2WwFi2xJw0czDA9whW2d6wheAEKsKmAXRmB12ufGPrRWurqQUMoGpYsOxWe9MDK3Ep3ClLvjQoeeeKznH1Gt8qWck60XkfG9vcS20fX9ik+8qMLhrDXdZDveMzyxvxa6sNy/E=
Message-ID: <26c14c2a0610240805kf8cdc86y375d805f450e29ca@mail.gmail.com>
Date: Tue, 24 Oct 2006 19:05:01 +0400
From: "Oleg Khaschansky" <oleg.v.khaschansky@gmail.com>
To: harmony-dev@incubator.apache.org
Subject: Re: [classlib][awt] Revision #465514 broke image decoders.
In-Reply-To: <200610241453.k9OErMfx002825@d06av02.portsmouth.uk.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <26c14c2a0610240625r4c973ab1ub588745f8ad6f65a@mail.gmail.com>
	 <3b3f27c60610240649w21781c0cj12cc562e6c2352e1@mail.gmail.com>
	 <3b3f27c60610240708v47c8bfa3lfc676c23fbd6d265@mail.gmail.com>
	 <26c14c2a0610240725t5918e3d5t51900ecc444dcdf3@mail.gmail.com>
	 <200610241453.k9OErMfx002825@d06av02.portsmouth.uk.ibm.com>

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

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