On Mon, 2 Dec 2024 16:55:23 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 22 additional >> commits since the last revision: >> >> - provide guidance to subclasses on which method to override for cleaning >> up resources >> - Revert "Roger's suggestion - Make Inflater.close() and Deflater.close() >> final, also update the new tests to match this change" >> >> This reverts commit b60181bbb4be9fac294b16820cd02017de71783e. >> - merge latest from master branch >> - update end() to remove mention of other methods throwing >> IllegalStateException >> - update the class level documentation of Inflater to match the updates in >> Deflater >> - merge latest from master branch >> - improve Deflater class level doc >> - Stuart's review - improve end() API doc >> - merge latest from master branch >> - missed a few methods for specifying IllegalStateException >> - ... and 12 more: https://git.openjdk.org/jdk/compare/ec8a7574...42ff9059 > > src/java.base/share/classes/java/util/zip/Inflater.java line 58: > >> 56: * To release resources used by the {@code Inflater}, applications must >> call the >> 57: * {@link #end()} method. After {@code end()} has been called, >> subsequent calls >> 58: * to several methods of the {@code Inflater} will throw an {@link >> IllegalStateException}. > > Since `close()` is not mentioned here, there may be some come confusion about > whether `end()` still needs to be called within T-W-R. I think that's a good point. I've now updated the PR to reword this part of the text both in `Inflater` and `Deflater`. Does the change look OK? > test/jdk/java/util/zip/DeflaterClose.java line 43: > >> 41: >> 42: /** >> 43: * Closes the Deflater multiple times and then expects close() and >> end() to be called that > > Should this be "close() *or* end()"? Hello Roger, calling `close()` on Inflater/Deflater will just call `end()` on the instance. So I believe "close() and end()" in the test comment here is correct. I now pushed a very minor change just to make it more clear. Let me know if you think we should improve the comment further. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19675#discussion_r1905158732 PR Review Comment: https://git.openjdk.org/jdk/pull/19675#discussion_r1905161112
