RFR: 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional GZIP fields [v3]

Lin Zang lzang at openjdk.java.net
Wed Mar 24 06:53:42 UTC 2021


On Mon, 22 Mar 2021 21:37:27 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> Lin Zang has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - update copyright
>>  - reuse arguments constructor for non-argument one.
>
> Hi Lin,
>  
> Again, thank you for taking on this 19+ year feature request.
> 
> I have not done a deep dive on the CSR, but wanted to get a few comments back to you on a quick scan of the last PR update.
> 
> Personally, I would like to see more testing for a change such as this given the age of the code:
> 
> - Please do not modify the existing test, you can either create  a new test(s) or add tests to the existing test class
> - We should capture Gzip files with these headers set from other tools and store the Gzip in an array within the test which can then be written to disk so the tests can validate interoperability.  Please see some of the other Zip tests for an example
> - We should have tests that include some , but not all of the additional fields as I believe they are all optional according to the RFC.
> - Please include some negative tests
> 
> 
> I have also include some additional comments within the code

Hi Lance,
Thanks a lot for your review. I will update the PR ASAP. 
May I ask your help to also review the CSR? Thanks!

BRs,
Lin

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

PR: https://git.openjdk.java.net/jdk/pull/3072


More information about the core-libs-dev mailing list