[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] RFR : 8200281: Add missing @Override annotations in ImageIO plugins
From:       Dmitry Markov <dmitry.markov () oracle ! com>
Date:       2020-07-30 10:19:04
Message-ID: B9657A38-988B-47FE-A50F-EA1B239B9D53 () oracle ! com
[Download RAW message or body]

Looks good to me.

Regards,
Dmitry

> On 30 Jul 2020, at 07:10, Kumar Abhishek <kumar.z.abhishek@oracle.com> wrote:
> 
> Hi Dmitry,
> 
> I have updated the year in the copyright section for the modified file.
> Please find the updated Webrev:-
> http://cr.openjdk.java.net/~jdv/8200281/webrev.03/
> 
> 
> Thanks,
> Abhishek
> -----Original Message-----
> From: Dmitry Markov 
> Sent: Wednesday, July 29, 2020 11:45 PM
> To: Kumar Abhishek <kumar.z.abhishek@oracle.com>
> Cc: Alexey Ivanov <alexey.ivanov@oracle.com>; Philip Race <philip.race@oracle.com>; \
>                 2d-dev <2d-dev@openjdk.java.net>
> Subject: Re: [OpenJDK 2D-Dev] RFR : 8200281: Add missing @Override annotations in \
> ImageIO plugins 
> Hi Abhishek,
> 
> Could you update the year in the copyright section for the modified files, please?
> 
> Regards,
> Dmitry
> 
> > On 29 Jul 2020, at 11:43, Kumar Abhishek <kumar.z.abhishek@oracle.com> wrote:
> > 
> > Hi Alexey/Dmitry,
> > 
> > Please find the updated Webrev.
> > http://cr.openjdk.java.net/~jdv/8200281/webrev.02/
> > 
> > I have added the missed annotation.
> > 
> > Thanks,
> > Abhishek
> > -----Original Message-----
> > From: Alexey Ivanov
> > Sent: Saturday, July 25, 2020 2:38 AM
> > To: Kumar Abhishek <kumar.z.abhishek@oracle.com>
> > Cc: Philip Race <philip.race@oracle.com>; 2d-dev 
> > <2d-dev@openjdk.java.net>
> > Subject: Re: [OpenJDK 2D-Dev] RFR : 8200281: Add missing @Override 
> > annotations in ImageIO plugins
> > 
> > Hi Abhishek,
> > 
> > *JPEGImageWriter.java*
> > 
> > 240     public IIOMetadata convertStreamMetadata(IIOMetadata inData,
> > 
> > 255     public IIOMetadata
> > 256         convertImageMetadata(IIOMetadata inData,
> > 
> > 1751         public synchronized void dispose() {
> > 
> > These methods are missed: no @Override annotation. In the last case, the method \
> > implements an interface rather than overrides a method from a super class, yet \
> > the @Override annotation is usually added. 
> > *BMPImageReader.java*
> > 1937                 public void imageProgress(ImageReader source,
> > 1945                 public void imageUpdate(ImageReader source,
> > 1956                 public void passComplete(ImageReader source,
> > 
> > 
> > So the patch is incomplete.
> > Please verify you add @Override annotation to all overridden methods in these \
> > classes. 
> > 
> > Regards,
> > Alexey
> > 
> > On 24/07/2020 18:59, Philip Race wrote:
> > > This is fine. Approved.
> > > 
> > > -phil.
> > > 
> > > On 7/24/20, 3:08 AM, Kumar Abhishek wrote:
> > > > 
> > > > Hi Phil,
> > > > 
> > > > This bug was originally only for JPEG plugin though I have verified 
> > > > the changes for other plugins as well.
> > > > 
> > > > Do you want me to divide the change for different plugins and create 
> > > > separate bugs or the current approach is fine?
> > > > 
> > > > Please find the updated webrev with superfluous comments removed :
> > > > 
> > > > http://cr.openjdk.java.net/~jdv/8200281/webrev.01/
> > > > <http://cr.openjdk.java.net/%7Ejdv/8200281/webrev.01/>
> > > > 
> > > > Thanks,
> > > > 
> > > > Abhishek
> > > > 
> > > > *From:*Philip Race
> > > > *Sent:* Thursday, July 23, 2020 10:37 PM
> > > > *To:* Kumar Abhishek <kumar.z.abhishek@oracle.com>
> > > > *Cc:* 2d-dev@openjdk.java.net
> > > > *Subject:* Re: [OpenJDK 2D-Dev] RFR : 8200281: Add missing @Override 
> > > > annotations in ImageIO plugins
> > > > 
> > > > 
> > > > 
> > > > On 7/23/20, 5:20 AM, Kumar Abhishek wrote:
> > > > 
> > > > Hello, Please review this small change for JDK-16 .
> > > > 
> > > > This is a clean up task to verify and add missing annotations in
> > > > ImageIO plugins
> > > > 
> > > > 
> > > > They are missing in part because most of these classes were added in
> > > > 1.4 and
> > > > annotations were introduced in 1.5. So I am not sure how much 
> > > > retrofitting we should do along these lines. If applied widely it 
> > > > could just make backports painful when patches will not apply.
> > > > 
> > > > 
> > > > /** Overrides the method defined in the superclass. */
> > > > +    @Override
> > > > 
> > > > The comment is now superflous so I suggest to remove it in the 
> > > > couple of places it is present.
> > > > 
> > > > -phil.
> > > > 
> > > > Verified and added annotations for all the Writer and Reader
> > > > implementation under
> > > > src/java.desktop/share/classes/com/sun/imageio/plugins/
> > > > 
> > > > Bug/webrev :
> > > > 
> > > > https://bugs.openjdk.java.net/browse/JDK-8200281
> > > > 
> > > > http://cr.openjdk.java.net/~jdv/8200281/webrev.00/  
> > > > <http://cr.openjdk.java.net/%7Ejdv/8200281/webrev.00/>
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > Thanks, Abhishek
> > > > 
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]