On Wed, 24 Mar 2021 06:51:16 GMT, Lin Zang <lz...@openjdk.org> wrote:
>> 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 Hi Lin, On Mar 24, 2021, at 2:51 AM, Lin Zang ***@***.******@***.***>> wrote: Hi Lance， Thanks a lot for your review. I will update the PR ASAP. May I ask your help to also review the CSR？ I believe we need to flush out some of the issues I raised that were not test related as they will result in updates to the javadoc which will require an update to the CSR. Thanks! BRs, Lin — You are receiving this because you commented. Reply to this email directly, view it on GitHub<https://github.com/openjdk/jdk/pull/3072#issuecomment-805549600>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABQPFO5R4INTAUFZ3QS6WL3TFGDXHANCNFSM4ZMLU6SA>. ***@***.*** Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 ***@***.******@***.***> ------------- PR: https://git.openjdk.java.net/jdk/pull/3072
