On Mon, 26 Jan 2026 18:13:46 GMT, Jorn Vernee <[email protected]> wrote:
>> Christian Stein has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address review comments > > src/jdk.jartool/share/classes/sun/tools/jar/Validator.java line 105: > >> 103: this.zis = zis; >> 104: checkModuleDescriptor(MODULE_INFO); >> 105: checkAutomaticModuleName(); > > The separate method is much better. I don't think doing this in the > constructor is great though. I suggest moving this, and the existing call to > `checkModuleDescriptor` to the `validate` method, then the constructor is for > instantiation only, and the validate method handles the validate logic. (but, > that's more of a personal preference) I agree, yet the existing `checkModuleDescriptor(MODULE_INFO);` method initializes the `this.md` field as a side-effect, maybe even more fields. I'd like to defer such cleanup work to a dedicated overhaul of the `jar` tool, or at least a refactoring of the `--validate` operation mode. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29316#discussion_r2731524384
