4206909 - adding Z_SYNC_FLUSH support to deflaters

[Martin Buchholz]

[+Brandon]

For the test copyright line, use

 * Copyright 2009 Google, Inc.  All Rights Reserved.

as in other test cases.

Whether, in addition, to have a Sun copyright depends on
whether you made "substantive modifications".
Heuristic for that is 10 lines of code.

Obviously, the Sun copyright got there via copy-n-paste
from existing files.

---

You still have @linke typos and lingering TABs to untabify.

---

Otherwise, I'm happy enough with this - I think we have rough consensus.

Martin

On Wed, Sep 23, 2009 at 10:08, Xueming Shen <Xueming.Shen at sun.com> wrote:
>
> Martin, thanks for the comments.
>
> webrev has been updated accordingly, I will submit it for the CCC.
>
> What should the "google copyright" look like for InflateIn_DeflateOut.java?
>
> I picked it from
>
> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/Z_SYNC_FLUSH/test/java/util/zip/InflateIn_DeflateOut.java.html
>
> which uses the sun copyright, I guess it was copied from the your
> DeflateIn_InflateOut.java test case.
>
> So if you can pass me your google version I will paste it in. I assume It
> should be OK that I then put the
> sun one on top of that.
>
> Glad to see the light at the end of the tunnel finally:-)
>
> sherman
>
> Martin Buchholz wrote:
>>
>> [+ Brandon]
>>
>> Sherman,
>> Here are some review comments on "take1"
>>
>> Overall, looks good.
>> This low-level API will be annoying to use,
>> but that's mostly unavoidable.
>>
>> ----
>> typos: @linke sinze
>> ----
>> source files contain TABs, no longer permitted
>> ----
>> <tt> is denigrated in favor of @code
>> ----
>> I don't think you really meant to include ./Flush.java in the webrev
>> ----
>> {@link Deflater.SYNC_FLUSH}
>> =>
>> {@link Deflater#SYNC_FLUSH}
>> ----
>> Please use a Google copyright in InflateIn_DeflateOut
>> ----
>>
>> Martin
>>
>> On Tue, Sep 22, 2009 at 13:03, Xueming Shen <Xueming.Shen at sun.com> wrote:
>>
>>>
>>> Thanks Alan!
>>>
>>> The webrev has been updated accordingly to address your comments (use
>>> syncFlush instead of
>>> doSyncFlush and those suggestions for Deflater.java).
>>>
>>> http://cr.openjdk.java.net/~sherman/zipflush/webrev
>>>
>>> Sherman
>>>
>>> PS. There was a "take2" for DOS that I think might be more consistent
>>> with
>>> the existing APIs, but it might be the
>>> time to focus on one approach. If you're interested the "take2" is at
>>>
>>>
>>> http://cr.openjdk.java.net/~sherman/zipflush/webrev.take2/src/share/classes/java/util/zip/DeflaterOutputStream.java.sdiff.html
>>>
>>> Alan Bateman wrote:
>>>
>>>>
>>>> Welcome back (for some reason I thought you were gone for two weeks).
>>>>
>>>> It would be best to send the proposal to the mailing list so that others
>>>> can comment.
>>>>
>>>> Personally, I don't like exposing the flush mode to subclasses but you
>>>> are
>>>> right that it is more consistent with the original design.  If this
>>>> approach
>>>> is chosen then I would suggest that the flush implementation make a copy
>>>> of
>>>> the flush mode before testing and using it. Also, the new constructors
>>>> will
>>>> need to say that they initialize the flush mode based on the parameter
>>>> (since it can change on the fly). BTW: What is the reason for renaming
>>>> the
>>>> parameter to flushDef. I prefer "syncFlush" over "doSyncFlush".
>>>>
>>>> I'm happy with Deflater. In the description of SYNC_FLUSH I would
>>>> suggest
>>>> changing the "," into a ";" or else make it into two sentences. In the
>>>> new
>>>> deflate method the description of NO_FLUSH needs a comma after
>>>> "accumulate"
>>>> for it to flow well. Similar issue in the SYNC_FLUSH description where
>>>> you
>>>> need a comma after "is flushed". There is also a typo [ comparssed :-) ]
>>>>
>>>> -Alan.
>>>>
>>>>
>>>>
>>>
>>>
>
>