RFR: 8331051: Add an `@since` checker test for `java.base` module [v4]

Nizar Benalla duke at openjdk.org
Sun May 5 14:29:19 UTC 2024


On Thu, 2 May 2024 13:49:43 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

>> Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   - Added some legacy modules that existed long before preview features (they were incubating)
>>   - Not checking elements enclosed withing a record
>>   - Only check if the file is readable using `Files.isReadable`
>>   - Dropped the use of `Files.exists` and `Files.isDirectory`
>>   - Use `--add-modules` option now to resolve certain modules
>
> test/jdk/tools/sincechecker/SinceChecker.java line 51:
> 
>> 49: 
>> 50: /*
>> 51: - This checker checks the values of the `@since` tag found in the documentation comment for an element against the release in which the element first appeared.
> 
> In addition to the long lines mentioned by Jon which make the comments hard to read, I find it strange that every sentence is formatted as a list item with a leading dash. I think it's ok when describing different parts/steps of the algorithm, but at least the first sentence in the comment should not be a list item IMO.

Fixed, thanks.

> test/jdk/tools/sincechecker/SinceChecker.java line 216:
> 
>> 214:             srcZip = testJdk.getParent().resolve("images").resolve("jdk").resolve("lib").resolve("src.zip");
>> 215:         }
>> 216:         if (!Files.exists(srcZip) && !Files.isDirectory(srcZip)) {
> 
> This condition looks wrong. If `exists` returns false, it also implies that `isDirectory` returns false. I think there's no need to check for a (not-)directory here.

Fixed in [3f226ef](https://github.com/openjdk/jdk/pull/18934/commits/3f226ef9134b71a1b63b6562b8381be909c30343), I now only check if the file is readable.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1590327624
PR Review Comment: https://git.openjdk.org/jdk/pull/18934#discussion_r1590327722


More information about the core-libs-dev mailing list