|
From: | Mark Wielaard |
Subject: | Re: Patches to java.util.zip by Christian Schlichtherle |
Date: | Tue, 30 Aug 2005 18:02:30 +0200 |
Hi, On Sun, 2005-07-24 at 12:29 +0700, John Leuner wrote: > Unfortunately I haven't been able to give this task the attention it > deserves and I want to give someone else the opportunity to deal with > these patches. > > Below are some of the emails exchanged between Christian, Mark Wielaard > and myself. I went through what I reviewed earlier and committed all uncontroversial stuff. > > I didn't actually review the big part of the patch. And I don't think we > > should add new public/protected constructors or methods. We should probably > > move the new functionality to a new gnu.java.util.zip package with extended > > functionality if possible. > > > > But I quickly went through some of the things that I could quickly format > > correctly and that I think can and should be submitted separately since they > > are good changes on their own anyway. > > > > Reformatted Adler32.java to follow out style guide and make the diff minimal > > and updated copyright year. This can probably be submitted separately with a > > little ChangeLog message plus a short explanation of the why of the changed > > (any benchmarks?) I haven't done any benchmarks myself. We probably should also have a little mauve tests for Adler32 to make sure we don't introduce any bugs. Cleaned up patch attached. > > Dropped the change to java/util/zip/CRC32.java which I didn't understand > > (what is CRC32If?): > > -public class CRC32 implements Checksum > > +public class CRC32 implements CRC32If > > > > Same for the changes to java/util/zip/Deflater.java (what is > > DeflaterIf?) > > > > Dropped the change to java/util/zip/DeflaterEngine.java private methods are > > always final: > > - private void updateHash() { > > + private final void updateHash() { > > > > Added copyright year to java/util/zip/DeflaterHuffman.java and changed order > > of modifiers to amtch coding styleguide. (Can also be submitted separately > > as obvious fix with ChangeLog entry.) Commited as: 2005-08-30 Christian Schlichtherle <address@hidden> * java/util/zip/DeflaterHuffman.java (bit4Reverse): Mark final. > > Removed the changes to java/util/zip/DeflaterOutputStream.java that involved > > DeflaterIf (which wasn't included and we cannot change the type and modifier > > of a protected field). Are you sure you want to comment out/remove the > > flush() method? (Can also be submitted separately with its own ChangeLog > > entry.) Dropped the removal of flush() but kept the increased buffer size. Committed as: 2005-08-30 Christian Schlichtherle <address@hidden> * java/util/zip/DeflaterOutputStream.java (DeflaterOutputStream(OutputStream)): Increase buffer size to 4096. (DeflaterOutputStream(OutputStream,Deflater)): Likewise. > > Updated the copyright year in java/util/zip/Inflater.java and removed the > > implements InflaterIf. Updated documentation is obviously correct and can be > > submitted separately with a ChangeLog entry. A similar fix was already applied earlier: 2005-07-13 David Gilbert <address@hidden> * java/util/zip/Inflater.java: minor API doc fixes. > > Remove the changes to java/util/zip/InflaterInputStream.java which all had > > to do with InflaterIf (not included). > > > > The change to java/util/zip/OutputWindow.java is unnecessary since private > > methods are already final: > > - private void slowRepeat(int rep_start, int len, int dist) > > + private final void slowRepeat(int rep_start, int len, int dist) > > > > java/util/zip/ZipException.java had as only change an update to the > > copyright years list. That is unnecessary. I need to go through the rest of these patches. It would help if they could be rediffed against current CVS head and extra functionality would be moved to a new package. Thanks, Mark
Adler32.patch
Description: Text Data
DeflaterHuffman.patch
Description: Text Data
DeflaterOutputStream.patch
Description: Text Data
signature.asc
Description: This is a digitally signed message part
[Prev in Thread] | Current Thread | [Next in Thread] |