> Can I please get a review of this change which proposes to remove the > (internal) `SelectVersion()` function from the java launcher and also update > the code comments in the launcher to match the current implementation? > > As noted in https://bugs.openjdk.org/browse/JDK-8340114, the > `SelectVersion()` function in the launcher used to be relevant when JRE > selection was a feature. That feature has been removed since Java 9 > https://bugs.openjdk.org/browse/JDK-8058407. The SelectVersion() in its > current form isn't relevant anymore and can be removed. > > While at it, it was noticed that the current "flowchart" code comments in the > launcher which attempts to explain the flow in the launcher code are > outdated. The commit in this PR updates those comments for macosx and unix > implementation. The windows variant doesn't have a "flowchart", but it too > deserves a high level comment explaining this flow. I haven't updated the > windows variant in this PR because that does a few additional things, which I > need to review and understand better. I plan to take that up in a future > change. > > An existing > `test/jdk/tools/launcher/MultipleJRERemoved.java/MultipleJRERemoved` test had > to be updated due to the changes in this PR. That test was launching `java` > (once) with 3 unsupported JRE selection options and was expecting 3 error > messages (one each for the unsupported option) for that single launch. With > the change in this PR, we don't accumulate and throw all those 3 errors and > instead we fail fast for any of these 3 unsupported JRE selection options. > The fail fast implementation matches what we do with other similar > unsupported options. The test had to be updated to not expect all 3 errors > message in a single launch and instead expect to find one of those error > messages. Given what this test is for, and the fact that JRE version > selection options (rightly) continue to raise an error after this change, I > think, an update to that test should be OK. > > No new tests have been introduced in this PR and tier testing is currently in > progress.
Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: fix code comment style where appropriate ------------- Changes: - all: https://git.openjdk.org/jdk/pull/20997/files - new: https://git.openjdk.org/jdk/pull/20997/files/0db4faf3..84135eff Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20997&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20997&range=01-02 Stats: 25 lines in 3 files changed: 0 ins; 0 del; 25 mod Patch: https://git.openjdk.org/jdk/pull/20997.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20997/head:pull/20997 PR: https://git.openjdk.org/jdk/pull/20997
