Hello Lance, On 10/07/19 2:25 AM, Lance Andersen wrote: > ——— > @implSpec This method is a no-op if this compressor has already > 886 * been previously closed, > > ———— > > Please remove “already” in both the close() and end() methods.
Done. > I believe the preference is the @implSpec and its relatives are on > their own line as in https://openjdk.java.net/jeps/8068562 and was > done for @apiNote earlier Done. > > Outside of the @ImplSpec, I am not sure the initial wording for end() > and close() really need to differ: > > end(): > > Closes the decompressor and discards any unprocessed input. > > close(): > > Releases resources held by this decompressor and discards any > unprocessed input.This method should be called when the > decompressor is no longer needed > > I have now updated the javadoc of end() to be closer to the javadoc of close(). > >> - The javadoc of end() method in both these classes has been updated to >> encourage the use of close() method instead of this one. It now also has >> a @implSpec which states that it's a no-op if called more than once. >> >> In addition, this javadoc has also been updated to replace the >> "undefined behaviour" statement with a mention that the usage of the >> Inflater/Deflater instance after a call to end() may throw an exception >> in those subsequent usages. Please note that, there's no such explicit >> mention in the javadoc of the (newly added) close() method because IMO, >> it isn't needed for close() since I think it's kind of implied that a >> closed resource can no longer be used for further operations. > > We need to be specific in close() also for clarity I haven't updated this in the latest webrev version and will wait for us to come to a decision on how we word it for end(). >> >> >> - TotalInOut.java test has been updated to use the new >> try-with-resources construct for the inflater and deflater it uses. > > Please update @biug to include the bug number Done. >> >> - A couple of new (testng) test classes have been added to test various >> scenarios where close() and end() method get called. These test mostly >> focus on ensuring that the close() and end(), either implicitly or >> explicitly, get called the right number of times on the subclasses of >> Inflater/Deflater. > > Overall they look OK. In your tests, you are testing the number of > calls for the sub-classes not for Deflate/Inflate so I would either > update your comments to clarify that or pull them into their own test > methods > I did not understand this. Did you mean I should update the @summary part of these tests or was it the javadoc on these test methods? The latest webrev with the above noted changes is available at http://cr.openjdk.java.net/~jpai/webrev/8225763/3/webrev/ -Jaikiran
