On Sat, 7 Mar 2026 05:46:21 GMT, Jaikiran Pai <[email protected]> wrote:

>> Jan Lahoda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Removing an unnecessary dependency as suggested.
>
> make/langtools/tools/previewfeature/SetupPreviewFeature.java line 77:
> 
>> 75:             }
>> 76:             String sourceCode = Files.readString(source);
>> 77:             var target = Paths.get(args[1]);
> 
> Nit - we have been using `Path.of(...)` in new code. `Paths.get(...)` merely 
> calls `Path.of(...)`.

Done, thanks!

> make/langtools/tools/previewfeature/SetupPreviewFeature.java line 88:
> 
>> 86:                                                                         
>> "/*compatibility constants:*/ ",
>> 87:                                                                         
>> ",\n")));
>> 88:                     out.write(sourceCode, insertPosition, 
>> sourceCode.length() - insertPosition);
> 
> In theory, `insertPosition` could be `-1` if the JDK source code for 
> `PreviewFeature.Feature` enum had no enum values. But I suspect that's not 
> going to happen because I see that we have a `TEST` enum value which I think 
> will never be removed:
> 
> /**
>  * A key for testing.
>  */
> @JEP(number=2_147_483_647, title="Test Feature")
> TEST,
> 
> So I think this is OK and we don't need to do any explicit checks for 
> insertPosition.

Yes, I believe we have two perpetual keys - `TEST` and `LANGUAGE_MODEL`, so we 
always should have some position to use.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30072#discussion_r2903805117
PR Review Comment: https://git.openjdk.org/jdk/pull/30072#discussion_r2903810042

Reply via email to