On Wed, 31 May 2023 18:02:09 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:
>> Joe Darcy has updated the pull request with a new target base due to a merge >> or a rebase. The pull request now contains 24 commits: >> >> - Merge branch 'master' into JDK-8306584 >> - Merge branch 'master' into JDK-8306584 >> - Merge branch 'master' into JDK-8306584 >> - Sync in symbol changes for JDK 21 build 24. >> - Merge branch 'master' into JDK-8306584 >> - Merge branch 'master' into JDK-8306584 >> - Merge branch 'master' into JDK-8306584 >> - Minor test fixes. >> - Merge branch 'master' into JDK-8306584 >> - Update symbol files to JDK 21 b23. >> - ... and 14 more: https://git.openjdk.org/jdk/compare/cb40db05...376dbe26 > > src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassFile.java line > 127: > >> 125: V64(64, 0), // JDK 20 >> 126: V65(65, 0), // JDK 21 >> 127: V66(66, 0); // JDK 22 > > A very small nit/suggestion - it should be possible to do: > > V66(66, 0), //JDK 22 > ; > > > (i.e. ending the enum constant with `,`, and putting the semicolon on a new > line.) This way adding a new constant would mean just one line addition, no > modification. (The same could be done for other enums.) Updated. > test/langtools/tools/javac/versions/Versions.java line 93: > >> 91: TWENTY(false, "64.0", "20", Versions::checksrc20), >> 92: TWENTY_ONE(false,"65.0", "21", Versions::checksrc21), >> 93: TWENTY_TWO(false,"66.0", "22", Versions::checksrc21); > > Should there be `checksrc22` instead of `checksrc21`? Or is that done later? Good catch. I have a refactoring of the test planned which will eliminate the explicit "checksrc$N" methods. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13567#discussion_r1212284124 PR Review Comment: https://git.openjdk.org/jdk/pull/13567#discussion_r1212283880
