On Fri, 26 Mar 2021 02:14:54 GMT, Lin Zang <lz...@openjdk.org> wrote:
>> 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 >> ***@***.******@***.***> > > Dear Lance， > > Sorry that I reply so late, I got stuck at some work recently. > >> * 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. > > I see Joe's comments in the CSR about the encode of the byte array of > String-alike field such file name, I checked that the RFC has description it > is "ISO8859-1". So do you think it is ok to change the argument type to > String and add the encoding decription in the documented comments? > > And Joe also suggested to make all optional header field as a class (I > propose to name it "gzipHeaderMetaRecord" ). And then the constructor could > accept it and process related flag and fields. Moreover it seems this class > cloud also be used in GzipInputStream，Do you think it is ok to also change it > here? Thanks! > > > BRs, > Lin GZIPInputStream/GZIPOutputStream are JDK 1.1 era classes that aren't well specified and I think we'll have to do some improvements as part of extending the support. For example, the GZIPOutputStream constructors (existing and proposed) do not specify that they write the member header. As regards the new constructors then I think new parameters will need to be clearly specified and/or linked to their description in the RFC. The types of some of the parameters may need to be re-examined, maybe the file name and comment should be provided as Strings (that will help with the discussion about the encoding to 8859_1). I think the javadoc will need to make it clear on what flags/values are allowed and the behavior when other values are used. It might be that the class will need enums and other classes to provide a better API. ------------- PR: https://git.openjdk.java.net/jdk/pull/3072
