Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(508)

Issue 12421043: code review 12421043: archive/zip: allow user-extensible compression methods

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by Dustin
Modified:
10 years, 7 months ago
Reviewers:
rsc, bradfitz
CC:
golang-dev, bradfitz, rsc
Visibility:
Public.

Description

archive/zip: allow user-extensible compression methods This change replaces the hard-coded switch on compression method in zipfile reader and writer with a map into which users can register compressors and decompressors in their init()s.

Patch Set 1 #

Patch Set 2 : diff -r d20b285974a4 http://go.googlecode.com/hg/ #

Patch Set 3 : diff -r d20b285974a4 http://go.googlecode.com/hg/ #

Patch Set 4 : diff -r d20b285974a4 http://go.googlecode.com/hg/ #

Patch Set 5 : diff -r d20b285974a4 http://go.googlecode.com/hg/ #

Total comments: 11

Patch Set 6 : diff -r 6dee0694f35b http://go.googlecode.com/hg/ #

Total comments: 4

Patch Set 7 : diff -r 6dee0694f35b http://go.googlecode.com/hg/ #

Total comments: 3

Patch Set 8 : diff -r 7500ec6996b7 http://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -19 lines) Patch
M src/pkg/archive/zip/reader.go View 1 2 3 4 5 6 7 2 chunks +3 lines, -8 lines 0 comments Download
A src/pkg/archive/zip/register.go View 1 2 3 4 5 6 7 1 chunk +71 lines, -0 lines 0 comments Download
M src/pkg/archive/zip/writer.go View 1 2 3 4 5 6 7 2 chunks +7 lines, -11 lines 0 comments Download

Messages

Total messages: 15
Dustin
Hello golang-dev@googlegroups.com, I'd like you to review this change to http://go.googlecode.com/hg/
10 years, 8 months ago (2013-08-03 23:18:29 UTC) #1
bradfitz
https://codereview.appspot.com/12421043/diff/8001/src/pkg/archive/zip/reader.go File src/pkg/archive/zip/reader.go (right): https://codereview.appspot.com/12421043/diff/8001/src/pkg/archive/zip/reader.go#newcode47 src/pkg/archive/zip/reader.go:47: // Decompressor is a function that wraps a Reader ...
10 years, 7 months ago (2013-08-05 13:52:32 UTC) #2
Dustin
Thanks for the review. I uploaded a new changeset which hopefully resolves all these issues. ...
10 years, 7 months ago (2013-08-05 17:54:09 UTC) #3
bradfitz
Maybe update the consts too: const ( Store uint16 = 0 Deflate uint16 = 8 ...
10 years, 7 months ago (2013-08-05 18:03:23 UTC) #4
Dustin
Thanks so much for the doc help. I'm bad at Englishing. I don't think it ...
10 years, 7 months ago (2013-08-05 18:12:45 UTC) #5
bradfitz
LGTM but will let adg finish review & submit.
10 years, 7 months ago (2013-08-05 23:37:27 UTC) #6
bradfitz
Actually he's off in tropical paradise or something. I'll submit tomorrow morning if there are ...
10 years, 7 months ago (2013-08-05 23:38:26 UTC) #7
rsc
LGTM https://codereview.appspot.com/12421043/diff/18002/src/pkg/archive/zip/reader.go File src/pkg/archive/zip/reader.go (right): https://codereview.appspot.com/12421043/diff/18002/src/pkg/archive/zip/reader.go#newcode60 src/pkg/archive/zip/reader.go:60: if _, ok := decompressors[method]; ok { this ...
10 years, 7 months ago (2013-08-05 23:40:47 UTC) #8
Dustin
https://codereview.appspot.com/12421043/diff/18002/src/pkg/archive/zip/reader.go File src/pkg/archive/zip/reader.go (right): https://codereview.appspot.com/12421043/diff/18002/src/pkg/archive/zip/reader.go#newcode60 src/pkg/archive/zip/reader.go:60: if _, ok := decompressors[method]; ok { On 2013/08/05 ...
10 years, 7 months ago (2013-08-06 02:23:58 UTC) #9
bradfitz
https://codereview.appspot.com/12421043/diff/18002/src/pkg/archive/zip/reader.go File src/pkg/archive/zip/reader.go (right): https://codereview.appspot.com/12421043/diff/18002/src/pkg/archive/zip/reader.go#newcode53 src/pkg/archive/zip/reader.go:53: var decompressors = map[uint16]Decompressor{ make this: var ( demu ...
10 years, 7 months ago (2013-08-06 04:04:22 UTC) #10
Dustin
I did the register.go thing. I copied the header from another file and noticed after ...
10 years, 7 months ago (2013-08-06 05:09:06 UTC) #11
bradfitz
Looks like you haven't submitted a CLA. See http://golang.org/doc/contribute.html#copyright and let me know when that's ...
10 years, 7 months ago (2013-08-06 15:04:35 UTC) #12
Dustin
Sorry about that. I filled that the contributor thing with this gmail address (not the ...
10 years, 7 months ago (2013-08-06 17:25:46 UTC) #13
bradfitz
LGTM This turned out well. We can add more addresses to the CONTRIBUTORS file if ...
10 years, 7 months ago (2013-08-06 19:03:06 UTC) #14
bradfitz
10 years, 7 months ago (2013-08-06 19:03:41 UTC) #15
*** Submitted as https://code.google.com/p/go/source/detail?r=f9f44b2d4d79 ***

archive/zip: allow user-extensible compression methods

This change replaces the hard-coded switch on compression method
in zipfile reader and writer with a map into which users can
register compressors and decompressors in their init()s.

R=golang-dev, bradfitz, rsc
CC=golang-dev
https://codereview.appspot.com/12421043

Committer: Brad Fitzpatrick <bradfitz@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b