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.s...@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.
