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

Issue 97840044: code review 97840044: encoding/json: add example for Indent, clarify the docs.

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 11 months ago by smcquay
Modified:
5 years, 11 months ago
Reviewers:
adg, rh
CC:
golang-codereviews, minux1, bradfitz, aram, rh, r, adg
Visibility:
Public.

Description

encoding/json: add example for Indent, clarify the docs. There was confusion in the behavior of json.Indent; This change attempts to clarify the behavior by providing a bit more verbiage to the documentation as well as provide an example function. Fixes issue 7821.

Patch Set 1 #

Patch Set 2 : diff -r 824f981dd4b7 https://code.google.com/p/go/ #

Patch Set 3 : diff -r 824f981dd4b7 https://code.google.com/p/go/ #

Total comments: 4

Patch Set 4 : diff -r c19d7fd53785 https://code.google.com/p/go/ #

Patch Set 5 : diff -r c19d7fd53785 https://code.google.com/p/go/ #

Patch Set 6 : diff -r c19d7fd53785 https://code.google.com/p/go/ #

Patch Set 7 : diff -r c19d7fd53785 https://code.google.com/p/go/ #

Patch Set 8 : diff -r b0443478e712 https://code.google.com/p/go/ #

Patch Set 9 : diff -r b0443478e712 https://code.google.com/p/go/ #

Total comments: 4

Patch Set 10 : diff -r 2e591e82a8c8 https://code.google.com/p/go/ #

Patch Set 11 : diff -r 2e591e82a8c8 https://code.google.com/p/go/ #

Total comments: 10

Patch Set 12 : diff -r 2a7aa0e6641d https://code.google.com/p/go/ #

Patch Set 13 : diff -r e3d533e55750 https://code.google.com/p/go/ #

Total comments: 2

Patch Set 14 : diff -r e3d533e55750 https://code.google.com/p/go/ #

Patch Set 15 : diff -r e3d533e55750 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -2 lines) Patch
M src/pkg/encoding/json/example_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +32 lines, -0 lines 0 comments Download
M src/pkg/encoding/json/indent.go View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +3 lines, -2 lines 0 comments Download

Messages

Total messages: 24
smcquay
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
5 years, 11 months ago (2014-04-30 06:20:51 UTC) #1
minux1
please change the first line of the description to: encoding/json: add example for Indent, clarify ...
5 years, 11 months ago (2014-04-30 06:29:05 UTC) #2
bradfitz
https://codereview.appspot.com/97840044/diff/40001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/40001/src/pkg/encoding/json/example_test.go#newcode148 src/pkg/encoding/json/example_test.go:148: out.Write([]byte("ʕ◔ϖ◔ʔ{\nʕ◔ϖ◔ʔ\t\"destinations\": ")) We think this is too cute. Remove ...
5 years, 11 months ago (2014-04-30 14:29:22 UTC) #3
aram
> please change the first line of the description to: > encoding/json: add example for ...
5 years, 11 months ago (2014-04-30 14:32:03 UTC) #4
smcquay
On 2014/04/30 06:29:05, minux wrote: > please change the first line of the description to: ...
5 years, 11 months ago (2014-05-01 04:48:00 UTC) #5
smcquay
On 2014/04/30 14:29:22, bradfitz wrote: > We think this is too cute. Remove the emojicons. ...
5 years, 11 months ago (2014-05-01 04:50:35 UTC) #6
smcquay
On 2014/04/30 14:32:03, aram wrote: > Apart from what minux said, please run fmt(1) on ...
5 years, 11 months ago (2014-05-01 04:51:20 UTC) #7
rh
On 2014/05/01 04:51:20, smcquay wrote: > On 2014/04/30 14:32:03, aram wrote: > > Apart from ...
5 years, 11 months ago (2014-05-01 12:37:54 UTC) #8
aram
Yes, CL description, there are no hard limits for Go code. If it feels right, ...
5 years, 11 months ago (2014-05-01 12:40:28 UTC) #9
smcquay
Hello golang-codereviews@googlegroups.com, minux.ma@gmail.com, bradfitz@golang.org, aram@mgk.ro, robert.hencke@gmail.com (cc: golang-codereviews@googlegroups.com), Please take another look.
5 years, 11 months ago (2014-05-02 04:54:46 UTC) #10
rh
LGTM - my comments here are just alternatives, not requests. https://codereview.appspot.com/97840044/diff/160001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/160001/src/pkg/encoding/json/example_test.go#newcode149 ...
5 years, 11 months ago (2014-05-02 12:17:15 UTC) #11
r
https://codereview.appspot.com/97840044/diff/160001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/160001/src/pkg/encoding/json/example_test.go#newcode149 src/pkg/encoding/json/example_test.go:149: // the previous write did not end with newline, ...
5 years, 11 months ago (2014-05-02 14:41:37 UTC) #12
smcquay
Hello golang-codereviews@googlegroups.com, minux.ma@gmail.com, bradfitz@golang.org, aram@mgk.ro, robert.hencke@gmail.com, r@golang.org (cc: golang-codereviews@googlegroups.com), Please take another look.
5 years, 11 months ago (2014-05-03 07:05:03 UTC) #13
smcquay
Addressed requests. https://codereview.appspot.com/97840044/diff/40001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/40001/src/pkg/encoding/json/example_test.go#newcode148 src/pkg/encoding/json/example_test.go:148: out.Write([]byte("ʕ◔ϖ◔ʔ{\nʕ◔ϖ◔ʔ\t\"destinations\": ")) On 2014/04/30 14:29:22, bradfitz wrote: ...
5 years, 11 months ago (2014-05-03 07:09:16 UTC) #14
rh
LGTM ʕ◔ϖ◔ʔ
5 years, 11 months ago (2014-05-03 12:45:22 UTC) #15
adg
https://codereview.appspot.com/97840044/diff/200001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/200001/src/pkg/encoding/json/example_test.go#newcode155 src/pkg/encoding/json/example_test.go:155: // Output: Wouldn't it be clearer to write an ...
5 years, 11 months ago (2014-05-05 01:53:31 UTC) #16
smcquay
https://codereview.appspot.com/97840044/diff/200001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/200001/src/pkg/encoding/json/example_test.go#newcode155 src/pkg/encoding/json/example_test.go:155: // Output: On 2014/05/05 01:53:31, adg wrote: > Wouldn't ...
5 years, 11 months ago (2014-05-07 03:41:34 UTC) #17
adg
https://codereview.appspot.com/97840044/diff/200001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/200001/src/pkg/encoding/json/example_test.go#newcode155 src/pkg/encoding/json/example_test.go:155: // Output: On 2014/05/07 03:41:34, smcquay wrote: > On ...
5 years, 11 months ago (2014-05-08 04:00:50 UTC) #18
smcquay
Addressing adg's review. https://codereview.appspot.com/97840044/diff/200001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/200001/src/pkg/encoding/json/example_test.go#newcode155 src/pkg/encoding/json/example_test.go:155: // Output: On 2014/05/08 04:00:50, adg ...
5 years, 11 months ago (2014-05-08 06:24:26 UTC) #19
adg
Yeah that's much clearer to my eyes. https://codereview.appspot.com/97840044/diff/240001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/240001/src/pkg/encoding/json/example_test.go#newcode138 src/pkg/encoding/json/example_test.go:138: Road{"Diamond Fork", ...
5 years, 11 months ago (2014-05-08 06:26:52 UTC) #20
smcquay
Hello golang-codereviews@googlegroups.com, minux.ma@gmail.com, bradfitz@golang.org, aram@mgk.ro, robert.hencke@gmail.com, r@golang.org, adg@golang.org (cc: golang-codereviews@googlegroups.com), Please take another look.
5 years, 11 months ago (2014-05-08 06:28:05 UTC) #21
smcquay
*Actually* addressing adg's review. https://codereview.appspot.com/97840044/diff/240001/src/pkg/encoding/json/example_test.go File src/pkg/encoding/json/example_test.go (right): https://codereview.appspot.com/97840044/diff/240001/src/pkg/encoding/json/example_test.go#newcode138 src/pkg/encoding/json/example_test.go:138: Road{"Diamond Fork", 29}, On 2014/05/08 ...
5 years, 11 months ago (2014-05-08 06:42:15 UTC) #22
adg
LGTM Thanks for sticking with it!
5 years, 11 months ago (2014-05-08 06:45:37 UTC) #23
adg
5 years, 11 months ago (2014-05-08 06:52:48 UTC) #24
*** Submitted as https://code.google.com/p/go/source/detail?r=f7e221a34ec6 ***

encoding/json: add example for Indent, clarify the docs.

There was confusion in the behavior of json.Indent; This change
attempts to clarify the behavior by providing a bit more verbiage
to the documentation as well as provide an example function.

Fixes issue 7821.

LGTM=robert.hencke, adg
R=golang-codereviews, minux.ma, bradfitz, aram, robert.hencke, r, adg
CC=golang-codereviews
https://codereview.appspot.com/97840044

Committer: Andrew Gerrand <adg@golang.org>
Sign in to reply to this message.

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