Looks good to me. Regards, Dmitry
> On 30 Jul 2020, at 07:10, Kumar Abhishek <kumar.z.abhis...@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.abhis...@oracle.com> > Cc: Alexey Ivanov <alexey.iva...@oracle.com>; Philip Race > <philip.r...@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.abhis...@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.abhis...@oracle.com> >> Cc: Philip Race <philip.r...@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.abhis...@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 >>>> >> >
