Re: RFR: 8260959: remove RECORDS from PreviewFeature.Feature enum

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

Re: RFR: 8260959: remove RECORDS from PreviewFeature.Feature enum

Jonathan Gibbons-2
On Tue, 2 Feb 2021 18:33:18 GMT, Vicente Romero <[hidden email]> wrote:

> Please review this simple fix that is removing the RECORDS enum constant from the PreviewFeature.Feature enum, now that RECORDS are final in 16 this constant can be safely removed.
>
> Thanks,
> Vicente

Given the scary comment and reference to JDK 15 and boot cycle builds, is it really safe to remove the constant?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2360
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8260959: remove RECORDS from PreviewFeature.Feature enum

Joe Darcy-2
On Wed, 3 Feb 2021 01:12:19 GMT, Jonathan Gibbons <[hidden email]> wrote:

>> Please review this simple fix that is removing the RECORDS enum constant from the PreviewFeature.Feature enum, now that RECORDS are final in 16 this constant can be safely removed.
>>
>> Thanks,
>> Vicente
>
> Given the scary comment and reference to JDK 15 and boot cycle builds, is it really safe to remove the constant?

So currently, JDK 17 can be built with a boot JDK of 15, 16, or 17 (since JDK-8257459: "Bump minimum boot jdk to JDK 16") hasn't been fixed yet.

JDK 17 can currently be used as boot JDK only for itself, right? And in the future be used for JDK 18, and 19.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2360
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8260959: remove RECORDS from PreviewFeature.Feature enum

Vicente Romero-3
On Wed, 3 Feb 2021 01:37:28 GMT, Joe Darcy <[hidden email]> wrote:

>> Given the scary comment and reference to JDK 15 and boot cycle builds, is it really safe to remove the constant?
>
> So currently, JDK 17 can be built with a boot JDK of 15, 16, or 17 (since JDK-8257459: "Bump minimum boot jdk to JDK 16") hasn't been fixed yet.
>
> JDK 17 can currently be used as boot JDK only for itself, right? And in the future be used for JDK 18, and 19.

we already did a very similar fix for text blocks, so this patch is just doing the same for RECORDS

-------------

PR: https://git.openjdk.java.net/jdk/pull/2360
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8260959: remove RECORDS from PreviewFeature.Feature enum

Jan Lahoda-3
In reply to this post by Jonathan Gibbons-2
On Tue, 2 Feb 2021 18:33:18 GMT, Vicente Romero <[hidden email]> wrote:

> Please review this simple fix that is removing the RECORDS enum constant from the PreviewFeature.Feature enum, now that RECORDS are final in 16 this constant can be safely removed.
>
> Thanks,
> Vicente

I think that, in this specific case, we can remove the constant. The reason is that the PreviewFeature has been moved to a different package recently, so the JDK 15/16 versions of the API use the original PreviewFeature class, and should not produce any warnings. The builds look OK as well.

-------------

Marked as reviewed by jlahoda (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2360