On Mon, 28 Apr 2025 13:26:30 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:
>> This is a patch to finalize the module imports feature. Please see: >> https://bugs.openjdk.org/browse/JDK-8344700 > > Jan Lahoda has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 13 commits: > > - Merge branch 'master' into JDK-8344708 > - Adjusting ToolProviderTest to the updated persistence handling. > - Merge branch 'master' into JDK-8344708 > - Reflecting review feedback: cleanup formatting in ModuleInfo. > - Reflecting review feedback - avoiding hardcoded constants. > - Fixing test. > - Cleanup, fixing tests. > - Adjusting JShell defaults. > - Fixing tests. > - Cleanup - updated specification will permit requires transitive java.base; > for all classfile versions; java.se no longer participates in preview. > - ... and 3 more: https://git.openjdk.org/jdk/compare/1f228e55...da39d0ef src/jdk.jshell/share/classes/jdk/internal/jshell/tool/Startup.java line 345: > 343: boolean hasModuleImports = source == null || > 344: > Feature.MODULE_IMPORTS.allowedInSource(source); > 345: int idx = preview ? 2 : !hasModuleImports ? 1 : 0; It feels like this would be cleaner with an if/else if/else? I'm also not a big fan of hardcoded constants into an array. Maybe this code can benefit from stable values (when they are integrated). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23801#discussion_r2065991188
