RFR (T): 8243117: Cleanups in Java code of module jdk.jlink

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR (T): 8243117: Cleanups in Java code of module jdk.jlink

Langer, Christoph
Hi,

please help review this cleanup patch for the Java code in module jdk.jlink. It's mostly automatic IDE cleanups of unused imports plus a few removals for unused private methods, fields and annotations. I've also added some @Override annotations where such were missing.

Bug: https://bugs.openjdk.java.net/browse/JDK-8243117
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8243117.0/

Thanks
Christoph

Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink

Claes Redestad
Looks good to me, Christoph,

JmodTask.java: Not sure what the consensus is about wildcard imports,
but I'm fine with it here.

/Claes

On 2020-04-19 17:32, Langer, Christoph wrote:

> Hi,
>
> please help review this cleanup patch for the Java code in module jdk.jlink. It's mostly automatic IDE cleanups of unused imports plus a few removals for unused private methods, fields and annotations. I've also added some @Override annotations where such were missing.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8243117
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8243117.0/
>
> Thanks
> Christoph
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink

Alan Bateman


On 19/04/2020 17:31, Claes Redestad wrote:
> Looks good to me, Christoph,
>
> JmodTask.java: Not sure what the consensus is about wildcard imports,
> but I'm fine with it here.
Looks okay to me too, except @SuppressWarnings("unused") looks strange,
is that an Eclipse compiler warning name?

-Alan
Reply | Threaded
Open this post in threaded view
|

RE: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink

Langer, Christoph
In reply to this post by Claes Redestad
Hi Claes,

thanks for your quick review. I configured my IDE to go to wildcard at 10 imports from the same package. But if there's a common style guide for OpenJDK, I can certainly reconfigure this.

Best regards
Christoph

> -----Original Message-----
> From: Claes Redestad <[hidden email]>
> Sent: Sonntag, 19. April 2020 18:32
> To: Langer, Christoph <[hidden email]>; jigsaw-
> [hidden email]
> Cc: [hidden email]
> Subject: Re: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink
>
> Looks good to me, Christoph,
>
> JmodTask.java: Not sure what the consensus is about wildcard imports,
> but I'm fine with it here.
>
> /Claes
>
> On 2020-04-19 17:32, Langer, Christoph wrote:
> > Hi,
> >
> > please help review this cleanup patch for the Java code in module jdk.jlink.
> It's mostly automatic IDE cleanups of unused imports plus a few removals for
> unused private methods, fields and annotations. I've also added some
> @Override annotations where such were missing.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8243117
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8243117.0/
> >
> > Thanks
> > Christoph
> >
Reply | Threaded
Open this post in threaded view
|

RE: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink

Langer, Christoph
In reply to this post by Alan Bateman
Hi Alan,

> Looks okay to me too, except @SuppressWarnings("unused") looks strange,
> is that an Eclipse compiler warning name?

Yes, it comes from Eclipse, probably only used by its compiler. The private field EXIT_SYSERR doesn't seem to be used. Alternatively, I could comment it out? Or remove it?

/Christoph

Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink

Alan Bateman
On 19/04/2020 20:08, Langer, Christoph wrote:
> Hi Alan,
>
>> Looks okay to me too, except @SuppressWarnings("unused") looks strange,
>> is that an Eclipse compiler warning name?
> Yes, it comes from Eclipse, probably only used by its compiler. The private field EXIT_SYSERR doesn't seem to be used. Alternatively, I could comment it out? Or remove it?
>
The jimage tool went through a few iterations and I think it's just a
left over and should be okay to remove.

-Alan
Reply | Threaded
Open this post in threaded view
|

RE: RFR (T): 8243117: Cleanups in Java code of module jdk.jlink

Langer, Christoph
Hi Alan,

> >> Looks okay to me too, except @SuppressWarnings("unused") looks
> strange,
> >> is that an Eclipse compiler warning name?
> > Yes, it comes from Eclipse, probably only used by its compiler. The private
> field EXIT_SYSERR doesn't seem to be used. Alternatively, I could comment it
> out? Or remove it?
> >
> The jimage tool went through a few iterations and I think it's just a
> left over and should be okay to remove.

OK, I'll remove it altogether then: http://cr.openjdk.java.net/~clanger/webrevs/8243117.1/

I'll push it tomorrow, after a final round of testing.

Cheers
Christoph