On Thu, 31 Oct 2024 17:53:01 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Severin Gehwolf has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 172 commits: >> >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Some test fixes >> - Remove period in jlink.properties >> - Revert changes to ResourcePoolEntry >> - Fix comment in RuntimeImageLinkException >> - Remove ImageReader (like JmodsReader) >> - More comment fixes (JlinkTask) >> - Move some comments around >> - More comment fix-ups (JRTArchive) >> - Fix description of configure option >> - ... and 162 more: https://git.openjdk.org/jdk/compare/c40bb762...b702ba8c > > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 229: > >> 227: throw new >> RuntimeImageLinkException(msg); >> 228: } else { >> 229: // System.err vs taskHelper.warning? > > It's my leftover comment. I think this error or warning message should be > localized. But this would require to access `JlinkTask::taskHelper`. > This needs to consider what is the better way (possibly some other existing > cases too) and okay to leave it as is. > > Suggest to move this comment in line 224 and something like: > `// need to consider if this error or message should be localized` This should be localized, indeed. I've gone ahead and passed a taskHelper instance down to `JRTArchive` which should solve this problem. `RuntimeImageLinkException` now doesn't contain any localized strings anymore, but sufficient info to do the localization elsewhere. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1828187603
