On Mon, 1 Dec 2025 10:54:41 GMT, Alan Bateman <[email protected]> wrote:

>> Jaikiran Pai has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - move ensureOpen() to a few lines up
>>  - assert instead of ensureOpen()
>
> src/java.base/share/classes/jdk/internal/module/ModulePatcher.java line 295:
> 
>> 293:         }
>> 294: 
>> 295:         private void ensureOpen() throws IOException {
> 
> The private methods have a short description so you'll want to do the same 
> here.

Done.

> test/jdk/java/lang/module/ModuleReader/patched/PatchedModuleReaderTest.java 
> line 59:
> 
>> 57:                 .orElseThrow();
>> 58:         final ModuleReader reader;
>> 59:         final Stream<String> resources;
> 
> The use of finals on the locals to ensure a single assignment is a 
> distraction here. Once this test method is split once into small tests then 
> you can get rid of the finals.

I've updated the PR to refactor the test into more than one test method and 
also cleaned it up to remove the usage of `final`s.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2576935542
PR Review Comment: https://git.openjdk.org/jdk/pull/28569#discussion_r2576937567

Reply via email to