[go] compress/gzip: add examples

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai

Emmanuel Odeke uploaded a change:
https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples for:
+ NewWriter
+ NewReader
+ Writer.Reset
+ Reader.Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go
1 file changed, 63 insertions(+), 0 deletions(-)



diff --git a/src/compress/gzip/example_test.go
b/src/compress/gzip/example_test.go
new file mode 100644
index 0000000..2477c0c
--- /dev/null
+++ b/src/compress/gzip/example_test.go
@@ -0,0 +1,63 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package gzip_test
+
+import (
+ "bytes"
+ "compress/gzip"
+ "io"
+ "log"
+ "os"
+)
+
+func ExampleNewWriter() {
+ wc := gzip.NewWriter(os.Stdout)
+ defer wc.Close()
+ wc.Write([]byte("hello, world\n"))
+}
+
+func ExampleNewReader() {
+ buf := []byte{31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 202, 72, 205, 201,
201, 215, 81, 40, 207, 47, 202, 73, 225, 2, 4, 0, 0, 255, 255, 83, 116, 36,
244, 13, 0, 0, 0}
+ b := bytes.NewReader(buf)
+ rc, err := gzip.NewReader(b)
+ if err != nil {
+ log.Fatal(err)
+ }
+ defer rc.Close()
+
+ io.Copy(os.Stdout, rc)
+}
+
+func ExampleWriter_Reset() {
+ var buf bytes.Buffer
+ wc := gzip.NewWriter(&buf)
+ defer wc.Close()
+
+ wc.Write([]byte("hello, world\n"))
+
+ wc.Reset(os.Stdout)
+
+ wc.Write([]byte("Gophers\n"))
+}
+
+func ExampleReader_Reset() {
+ buf1 := []byte{31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 202, 72, 205, 201,
201, 215, 81, 40, 207, 47, 202, 73, 225, 2, 4, 0, 0, 255, 255, 83, 116, 36,
244, 13, 0, 0, 0}
+
+ rc, err := gzip.NewReader(bytes.NewReader(buf1))
+ if err != nil {
+ log.Fatal(err)
+ }
+ defer rc.Close()
+
+ io.Copy(os.Stdout, rc)
+
+ buf2 := []byte{31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 114, 207, 47, 200,
72, 45, 42, 230, 2, 4, 0, 0, 255, 255, 248, 97, 174, 38, 8, 0, 0, 0}
+ b2 := bytes.NewReader(buf2)
+ if err := rc.Reset(b2); err != nil {
+ log.Fatal(err)
+ }
+
+ io.Copy(os.Stdout, rc)
+}

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 1:

(4 comments)

https://go-review.googlesource.com/#/c/29218/1/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 15: func ExampleNewWriter() {
I would change this into a package-level example and use NewWriter and
NewReader in the same example function.

Also, I think this example should make use of gzip.Header. The Name,
ModTime, and Comment are probably the most frequently used header fields.
Keep in mind that the implementation is inconsistent in that the Reader
reads the header in NewReader or Reset, and the Writer writes the header at
the first Write or Close.


Line 17: defer wc.Close()
Since people have a tendency to copy-paste example code, we should be
consistent about error handling.

Both Close and Write return errors, so we should check them. Can you add
error checking here and all other cases?


PS1, Line 46: 31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 202, 72, 205, 201,

201, 215, 81, 40, 207, 47, 202, 73, 225, 2, 4, 0, 0, 255, 255, 83, 116, 36,
244, 13, 0, 0, 0

I'm not particularly fond of seeing hard-coded gzip data in the example. If
possible, we should craft the examples in such a way that this doesn't
show. My suggestion about Multistream below should avoid this.


PS1, Line 33: func ExampleWriter_Reset() {
: var buf bytes.Buffer
: wc := gzip.NewWriter(&buf)
: defer wc.Close()
:
: wc.Write([]byte("hello, world\n"))
:
: wc.Reset(os.Stdout)
:
: wc.Write([]byte("Gophers\n"))
: }
:
: func ExampleReader_Reset() {
: buf1 := []byte{31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 202,

72, 205, 201, 201, 215, 81, 40, 207, 47, 202, 73, 225, 2, 4, 0, 0, 255,
255, 83, 116, 36, 244, 13, 0, 0, 0}

:
: rc, err := gzip.NewReader(bytes.NewReader(buf1))
: if err != nil {
: log.Fatal(err)
: }
: defer rc.Close()
:
: io.Copy(os.Stdout, rc)
:
: buf2 := []byte{31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 114,

207, 47, 200, 72, 45, 42, 230, 2, 4, 0, 0, 255, 255, 248, 97, 174, 38, 8,
0, 0, 0}

: b2 := bytes.NewReader(buf2)
: if err := rc.Reset(b2); err != nil {
: log.Fatal(err)
: }
:
: io.Copy(os.Stdout, rc)
: }
Use of Reset is fairly straightforward in gzip. Instead of providing
examples for Reset, I would suggest providing an example for
Reader.Multistream, which is a clunky API to use.

In constructing a multistream gzip, you could probably make use of
Writer.Reset and kill 2 birds with one stone.

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>

Gerrit-HasComments: Yes

[Emmanuel Odeke (Gerrit)]

Emmanuel Odeke has posted comments on this change.

compress/gzip: add examples

Patch Set 1:

(4 comments)

Apologies for the late reply #dayJobCommitments

I've followed your suggestions and the update is coming up.
Writing the examples has given me some insight into using gzip, cool!

https://go-review.googlesource.com/#/c/29218/1/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 15: func ExampleNewWriter() {
> I would change this into a package-level example and use NewWriter and
> NewR

Gotcha. Nice, thanks for the recommendation.

Done!

Line 17: defer wc.Close()
> Since people have a tendency to copy-paste example code, we should be
> consi

Gotcha. Yeah, I didn't put in much error handling because in past example
CLs, error handling seemed to be doing a lot.

I totally agree, people copy and paste example code quite a lot.

Done, thanks!


PS1, Line 46: 31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 202, 72, 205, 201,

201, 215, 81, 40, 207, 47, 202, 73, 225, 2, 4, 0, 0, 255, 255, 83, 116, 36,
244, 13, 0, 0, 0

> I'm not particularly fond of seeing hard-coded gzip data in the example.
> If

Gotcha, yeah it felt awkward including hard-coded gzip data in there.

I followed your suggestions.

Thank you, done!

PS1, Line 33: func ExampleWriter_Reset() {
: var buf bytes.Buffer
: wc := gzip.NewWriter(&buf)
: defer wc.Close()
:

: wc.Write([]byte("hello, world\n"))
:
: wc.Reset(os.Stdout)
:
: wc.Write([]byte("Gophers\n"))
: }
:
: func ExampleReader_Reset() {
: buf1 := []byte{31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 202,

72, 205, 201, 201, 215, 81, 40, 207, 47, 202, 73, 225, 2, 4, 0, 0, 255,
255, 83, 116, 36, 244, 13, 0, 0, 0}

:
: rc, err := gzip.NewReader(bytes.NewReader(buf1))
: if err != nil {
: log.Fatal(err)
: }
: defer rc.Close()
:
: io.Copy(os.Stdout, rc)
:

: buf2 := []byte{31, 139, 8, 0, 0, 9, 110, 136, 0, 255, 114,

207, 47, 200, 72, 45, 42, 230, 2, 4, 0, 0, 255, 255, 248, 97, 174, 38, 8,
0, 0, 0}

: b2 := bytes.NewReader(buf2)
: if err := rc.Reset(b2); err != nil {
: log.Fatal(err)
: }
:
: io.Copy(os.Stdout, rc)
: }
> Use of Reset is fairly straightforward in gzip. Instead of providing
> exampl

Great suggestion. I've done that.

Thank you, done.


--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai

Emmanuel Odeke uploaded a new patch set:

https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concat and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go

1 file changed, 128 insertions(+), 0 deletions(-)

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai

Emmanuel Odeke uploaded a new patch set:

https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concat and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go

1 file changed, 134 insertions(+), 0 deletions(-)

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai

Emmanuel Odeke uploaded a new patch set:

https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concat and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go

1 file changed, 132 insertions(+), 0 deletions(-)

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 4:

(5 comments)

Thanks for writing up these examples.

https://go-review.googlesource.com/#/c/29218/4/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

PS4, Line 20: wc.Header.Comment = "gzip example"
: wc.Header.Name = "example-file"
: wc.Header.ModTime = time.Date(2006, time.February, 1, 3, 4,
5, 0, time.UTC)
Perhaps put a line-comment above this saying that setting the Header fields
is entirely optional.

Also, Header is an embedded struct. See we can just do wc.Comment, wc.Name,
and wc.ModTime.

What do you think of the following example data?

wc.Name = "a-new-hope.txt"
wc.Comment = "an epic space opera by George Lucas"
wc.ModTime = time.Date(1977, 5, 25, 0, 0, 0, 0, time.UTC)

And for the string to write:

"A long time ago in a galaxy far, far away..."


PS4, Line 48: header
No need for this. Just do rc.Comment, ...
Do the same in the multistream example.


PS4, Line 52: // Decompressed: Hello, Gophers!
: // Header fields:
: // Comment: gzip example
: // Name: example-file
: // ModTime: 2006-02-01 03:04:05 +0000 UTC
Thus, the output would be something like:

Name: a-new-hope.txt
Comment: an epic space opera by George Lucas
ModTime: 1977-05-25 00:00:00 +0000 UTC
Data: A long time ago in a galaxy far, far away...

It seems more natural when Name precedes Comment and also when the Data is
after all the "headers".


PS4, Line 63: file-1
Call it "file-1.txt" so it's obvious that Name is the filename. Some below.


PS4, Line 122: // File 1 Decompressed: Hello Gophers - 1
: // Header1 fields:
: // Comment: file-header-1
: // Name: file-1
: // ModTime: 2006-02-01 03:04:05 +0000 UTC
: // File 2 Decompressed: Hello Gophers - 2
: // Header2 fields:
: // Comment: file-header-2
: // Name: file-2
: // ModTime: 2007-03-02 04:05:06 +0000 UTC
Try to match the suggested output for the above example:

Name: file-1.txt
Comment: ...
ModTime: ...
Data: Hello Gophers - 1

Name: file-2.txt
Comment: ...
ModTime: ...
Data: Hello Gophers - 2

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>

Gerrit-HasComments: Yes

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 4:

(1 comment)

https://go-review.googlesource.com/#/c/29218/4/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

PS4, Line 122: // File 1 Decompressed: Hello Gophers - 1
: // Header1 fields:
: // Comment: file-header-1
: // Name: file-1
: // ModTime: 2006-02-01 03:04:05 +0000 UTC
: // File 2 Decompressed: Hello Gophers - 2
: // Header2 fields:
: // Comment: file-header-2
: // Name: file-2
: // ModTime: 2007-03-02 04:05:06 +0000 UTC
> Try to match the suggested output for the above example:

If you want, you can print newline when you Reset the reader so that the
output reads nicer.

[Emmanuel Odeke (Gerrit)]

Emmanuel Odeke has posted comments on this change.

compress/gzip: add examples

Patch Set 4:

(6 comments)

Thanks, done. Here comes another patch.

https://go-review.googlesource.com/#/c/29218/4/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

PS4, Line 20: wc.Header.Comment = "gzip example"
: wc.Header.Name = "example-file"
: wc.Header.ModTime = time.Date(2006, time.February, 1, 3, 4,
5, 0, time.UTC)
> Perhaps put a line-comment above this saying that setting the Header
> fields

Aha, nice. Pretty creative and engaging examples. SGTM.

Thanks for the reminder that Header is an embedded struct.

Done.

PS4, Line 48: header
> No need for this. Just do rc.Comment, ...

Done

PS4, Line 52: // Decompressed: Hello, Gophers!
: // Header fields:
: // Comment: gzip example
: // Name: example-file

: // ModTime: 2006-02-01 03:04:05 +0000 UTC

> Thus, the output would be something like:

Nice hint on the precedence of the fields for more natural readability.

Thanks, done!

PS4, Line 63: file-1
> Call it "file-1.txt" so it's obvious that Name is the filename. Some
> below.

Yap, done.

PS4, Line 122: // File 1 Decompressed: Hello Gophers - 1
: // Header1 fields:
: // Comment: file-header-1
: // Name: file-1
: // ModTime: 2006-02-01 03:04:05 +0000 UTC
: // File 2 Decompressed: Hello Gophers - 2
: // Header2 fields:
: // Comment: file-header-2
: // Name: file-2
: // ModTime: 2007-03-02 04:05:06 +0000 UTC
> Try to match the suggested output for the above example:

Cool, done.

PS4, Line 122: // File 1 Decompressed: Hello Gophers - 1
: // Header1 fields:
: // Comment: file-header-1
: // Name: file-1
: // ModTime: 2006-02-01 03:04:05 +0000 UTC
: // File 2 Decompressed: Hello Gophers - 2
: // Header2 fields:
: // Comment: file-header-2
: // Name: file-2
: // ModTime: 2007-03-02 04:05:06 +0000 UTC

> If you want, you can print newline when you Reset the reader so that the
> ou

Done

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai

Emmanuel Odeke uploaded a new patch set:

https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader

+ Reader.Multistream to concatenate and then

individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go

1 file changed, 129 insertions(+), 0 deletions(-)

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 5: Run-TryBot+1 Code-Review+2

(1 comment)

Thanks for the examples. I would wait for someone else on the Go team to
bless this before submitting.

https://go-review.googlesource.com/#/c/29218/5/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 21: // Setting the Header fields is entirely optional
nit: end with period.

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>

Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>
Gerrit-HasComments: Yes

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

compress/gzip: add examples

Patch Set 5:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=94677d85

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>
Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

compress/gzip: add examples

Patch Set 5: TryBot-Result+1

TryBots are happy.

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>

Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai, Gobot Gobot, Joe Tsai

Emmanuel Odeke uploaded a new patch set:

https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concatenate and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go

1 file changed, 129 insertions(+), 0 deletions(-)

[Emmanuel Odeke (Gerrit)]

Emmanuel Odeke has posted comments on this change.

compress/gzip: add examples

Patch Set 6:

Thanks for the reviews @dsnet. I've fixed the nit by adding a period to the
comment.

For sure, let me hala at @bradfitz for the extra person approval.

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>
Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>

Gerrit-HasComments: No

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick has posted comments on this change.

compress/gzip: add examples

Patch Set 6:

(3 comments)

https://go-review.googlesource.com/#/c/29218/6/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 21: // Setting the Header fields is entirely optional.
drop "entirely"


Line 63: // Setting the Header fields is entirely optional
drop "entirely" and end in a period


Line 95: rc.Multistream(false)
this is a little misleading since the example is named Multistream, yet
we're setting it false?


--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>

Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>
Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>

Gerrit-HasComments: Yes

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 6:

(1 comment)

https://go-review.googlesource.com/#/c/29218/6/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 95: rc.Multistream(false)
> this is a little misleading since the example is named Multistream, yet
> we'

I think it makes sense for this example to show up under the
Reader.Multistream method.

The default value of Multistream is true, so to actually read the file
one-stream-at-a-time, you have to set it to false.

Find it odd as well, but I find the multistream API a little in general.

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 6:

(1 comment)

https://go-review.googlesource.com/#/c/29218/6/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 95: rc.Multistream(false)

> I think it makes sense for this example to show up under the
> Reader.Multist

Last line: I find it odd as well, but I find the multistream API a little
odd in general.

[Emmanuel Odeke (Gerrit)]

Emmanuel Odeke has posted comments on this change.

compress/gzip: add examples

Patch Set 6:

(3 comments)

@bradfitz, updated the comments. Thanks.

https://go-review.googlesource.com/#/c/29218/6/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 21: // Setting the Header fields is entirely optional.
> drop "entirely"

Done.

Line 63: // Setting the Header fields is entirely optional
> drop "entirely" and end in a period

Done, thanks.

Line 95: rc.Multistream(false)
> this is a little misleading since the example is named Multistream, yet
> we'

Yeah, what @dsnet said -- in order to read one stream at a time we need to
set .Multstream(false).

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai, Gobot Gobot, Brad Fitzpatrick, Joe Tsai

Emmanuel Odeke uploaded a new patch set:

https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concatenate and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go

1 file changed, 129 insertions(+), 0 deletions(-)

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick has posted comments on this change.

compress/gzip: add examples

Patch Set 7:

(1 comment)

https://go-review.googlesource.com/#/c/29218/6/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 95: rc.Multistream(false)
> I think it makes sense for this example to show up under the
> Reader.Multist

Ah, the default is true. Okay, that's what I didn't know.

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>
Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>

Gerrit-HasComments: Yes

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick has posted comments on this change.

compress/gzip: add examples

Patch Set 7:

(1 comment)

https://go-review.googlesource.com/#/c/29218/7/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 103: if err := rc.Reset(&buf); err != nil {
the docs say "If there is no next stream, z.Reset(r) will return io.EOF."
but this code just explodes.

It should explicitly check for io.EOF here also.

Maybe this code should be a loop rather (looping from Multistream(false) to
fmt.Printf) rather than a bunch of duplicated code.

[Emmanuel Odeke (Gerrit)]

Emmanuel Odeke has posted comments on this change.

compress/gzip: add examples

Patch Set 7:

(1 comment)

https://go-review.googlesource.com/#/c/29218/7/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 103: if err := rc.Reset(&buf); err != nil {
> the docs say "If there is no next stream, z.Reset(r) will return io.EOF."
> b

Roger that, it was unrolled because of the way we started the example ie
with each file write on its own.
Yeah, that was my oversight on the io.EOF test. Done, thanks.

Yap, great suggestion, it looks nicer read in a loop and testing for io.EOF
to break the loop rather than with the duplicated code.
Done, thanks.

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai, Gobot Gobot, Brad Fitzpatrick, Joe Tsai

Emmanuel Odeke uploaded a new patch set:

https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concatenate and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go

1 file changed, 124 insertions(+), 0 deletions(-)

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick has posted comments on this change.

compress/gzip: add examples

Patch Set 8:

(3 comments)

https://go-review.googlesource.com/#/c/29218/8/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 35: rc, err := gzip.NewReader(&buf)
zr instead of rc.

rc is what it does (read closer), but zr is what it is (gzip reader)


Line 40: slurp, err := ioutil.ReadAll(rc)
I don't like encouraging slurping APIs. We should teach the streaming APIs.
What about:

https://play.golang.org/p/aFdVH7Wsyv


Line 50: fmt.Printf("Data: %s\n", slurp)
I'd drop Data: and put an extra \n before the data so it looks like RFC 822
headers.

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>
Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>

Gerrit-HasComments: Yes

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 8:

(2 comments)

https://go-review.googlesource.com/#/c/29218/8/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

PS8, Line 26: io.WriteString(wc,
While we're making other changes, let's avoid io.WriteString here. There is
no benefit to using it here. The allocation still exists, but it also
introduces another function call and also a type assertion. I'd like to
avoid having the examples portray WriteString as having a benefit here that
it doesn't have.

wc.Write([]byte(...))

Do this below too.

Line 50: fmt.Printf("Data: %s\n", slurp)
> I'd drop Data: and put an extra \n before the data so it looks like RFC
> 822

Hahaha, I think you've been HTTP land for too long ;P

The suggestion SGTM, though.

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 8:

(3 comments)

https://go-review.googlesource.com/#/c/29218/8/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

PS8, Line 19: wc
Perhaps wc should be zw to be consistent with Brad's suggestion below?


Line 60: var buf bytes.Buffer
Perhaps the Writer side should also be a loop?

See the archive/tar example to see what I mean:
https://golang.org/pkg/archive/tar/#example_


PS8, Line 94: defer rc.Close()
The Close method on gzip.Reader does absolutely nothing to cleanup
resources, but provides a way to sanity that the stream is complete without
errors.

Thus, a defer without checkout the error is pretty pointless.
I'm okay with this being at the end simply as:

if err := zr.Close(); err != nil {
log.Fatal(err)

[Emmanuel Odeke (Gerrit)]

Emmanuel Odeke has posted comments on this change.

compress/gzip: add examples

Patch Set 8:

(7 comments)

Sorry for the late replies #usualDayJobGrind

I've updated the examples

https://go-review.googlesource.com/#/c/29218/8/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

PS8, Line 19: wc
> Perhaps wc should be zw to be consistent with Brad's suggestion below?

True, done.

PS8, Line 26: io.WriteString(wc,
> While we're making other changes, let's avoid io.WriteString here. There
> is

Aye aye! Done, thanks.

Line 35: rc, err := gzip.NewReader(&buf)
> zr instead of rc.

Truu. Done, thanks.

Line 40: slurp, err := ioutil.ReadAll(rc)
> I don't like encouraging slurping APIs. We should teach the streaming
> APIs.

@bradfitz I see. However, that example seems like a bit of magic with the
headers plus body in the source itself.

Let me change up the examples to take out slurping. Am gonna prepare one
with io.Copy(os.Stdout, zr).

I was slurping because I assumed it would make easier to read the examples,
however, we can make them without the slurping.

Done, thanks.

Line 50: fmt.Printf("Data: %s\n", slurp)
> I'd drop Data: and put an extra \n before the data so it looks like RFC
> 822

Done

Line 60: var buf bytes.Buffer
> Perhaps the Writer side should also be a loop?

See I wanted to do that before, but thought I'd plainly add the parts in
unrolled. But sure, since it makes for better examples with less duplicated
code, done, thanks.

PS8, Line 94: defer rc.Close()
> The Close method on gzip.Reader does absolutely nothing to cleanup
> resource

Roger that. Done, thanks.

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai, Gobot Gobot, Brad Fitzpatrick, Joe Tsai

Emmanuel Odeke uploaded a new patch set:
https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concatenate and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go

1 file changed, 128 insertions(+), 0 deletions(-)

[Joe Tsai (Gerrit)]

Joe Tsai has posted comments on this change.

compress/gzip: add examples

Patch Set 9:

(1 comment)

Thanks for the changes. Overall, I'm happy with the examples.

https://go-review.googlesource.com/#/c/29218/9/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 18: var buf bytes.Buffer
Tiny nit: You use `var buf bytes.Buffer` here and you use `buf :=
new(bytes.Buffer)` below. Be consistent.

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>
Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>

Gerrit-HasComments: Yes

[Emmanuel Odeke (Gerrit)]

Emmanuel Odeke has posted comments on this change.

compress/gzip: add examples

Patch Set 9:

(1 comment)

https://go-review.googlesource.com/#/c/29218/9/src/compress/gzip/example_test.go
File src/compress/gzip/example_test.go:

Line 18: var buf bytes.Buffer
> Tiny nit: You use `var buf bytes.Buffer` here and you use `buf :=
> new(bytes

Ah I see, updating that.

Done, thanks.

[Emmanuel Odeke (Gerrit)]

Reviewers: Joe Tsai, Gobot Gobot, Brad Fitzpatrick, Joe Tsai

Emmanuel Odeke uploaded a new patch set:
https://go-review.googlesource.com/29218

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concatenate and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d
---
A src/compress/gzip/example_test.go
1 file changed, 128 insertions(+), 0 deletions(-)

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick has posted comments on this change.

compress/gzip: add examples

Patch Set 10: Run-TryBot+1 Code-Review+2

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>
Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

compress/gzip: add examples

Patch Set 10:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=5c5f47b6

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

compress/gzip: add examples

Patch Set 10: TryBot-Result+1

TryBots are happy.

[Brad Fitzpatrick (Gerrit)]

Brad Fitzpatrick has submitted this change and it was merged.

compress/gzip: add examples

Updates #16360.

Adds examples uing:
+ Writer, Reader
+ Reader.Multistream to concatenate and then
individually retrieve multiple gzipped files
+ Reset

Change-Id: I9ad9b92729a5cd58f7368eaf2db05f1cdf21063d

Reviewed-on: https://go-review.googlesource.com/29218
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
Reviewed-by: Joe Tsai <thebroke...@gmail.com>
Run-TryBot: Brad Fitzpatrick <brad...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>

---
A src/compress/gzip/example_test.go
1 file changed, 128 insertions(+), 0 deletions(-)

Approvals:
Brad Fitzpatrick: Looks good to me, approved; Run TryBots
Joe Tsai: Looks good to me, approved
Gobot Gobot: TryBots succeeded

[Emmanuel Odeke (Gerrit)]

Emmanuel Odeke has posted comments on this change.

compress/gzip: add examples

Patch Set 11:

Thanks for the reviews and patience @dsnet and @bradfitz!

--
https://go-review.googlesource.com/29218
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Emmanuel Odeke <emm....@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Joe Tsai <joe...@digital-static.net>
Gerrit-Reviewer: Joe Tsai <thebroke...@gmail.com>

Gerrit-HasComments: No