On Wed, 22 May 2024 11:26:02 GMT, Chen Liang <[email protected]> wrote:
>> Please review a simple patch to exclude preview visitor classes meant to
>> support future preview features from the Preview API page.
>>
>> The test adds an sample element annotated with the new
>> `PreviewFeature.Feature.LANGUAGE_MODEL` constant (which does not have a
>> `@JEP` annotation) to make sure it is not listed in the Preview API page.
>> The test itself does not have to be modified, as it would fail without the
>> change in `PreviewAPIListBuilder.java`.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/PreviewAPIListBuilder.java
> line 91:
>
>> 89: if (jep.number == 0) {
>> 90: // Remove preview support features without a valid JEP
>> 91: jeps.remove(feature);
>
> This feels like we are adding and removing a dummy jep to and from the map
> every time we encounter the language model feature. Can we just bind a dummy
> jep to that feature to avoid repeated processing, and only filter at the
> getter methods?
I agree it's not ideal. I think the best thing is to split up the insertion
into separate steps: First check if the key is contained in the map, retrieve
the JEP info if it isn't, and only then decide whether to create a JEP record.
But I found another problem with this PR, so there may be some further changes
coming.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19344#discussion_r1609881611
