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

Issue 6997045: code review 6997045: encoding/json: A JSON tag can be any valid JSON string. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 6 months ago by stephane.travostino
Modified:
9 years, 6 months ago
Reviewers:
CC:
golang-dev, DMorsing, remyoudompheng, rsc
Visibility:
Public.

Description

encoding/json: A JSON tag can be any valid JSON string. Fixes issue 3887.

Patch Set 1 #

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

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

Total comments: 2

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -1 line) Patch
M src/pkg/encoding/json/encode.go View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/encoding/json/tagkey_test.go View 1 2 3 4 2 chunks +10 lines, -0 lines 0 comments Download

Messages

Total messages: 13
stephane.travostino
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
9 years, 6 months ago (2012-12-22 01:26:34 UTC) #1
stephane.travostino
Please discard the modification to encode_test.go. I can't really figure out a way to discard ...
9 years, 6 months ago (2012-12-22 01:32:06 UTC) #2
DMorsing
On 2012/12/22 01:32:06, stephane.travostino wrote: > Please discard the modification to encode_test.go. > > I ...
9 years, 6 months ago (2012-12-22 09:45:52 UTC) #3
remyoudompheng
https://codereview.appspot.com/6997045/diff/5001/src/pkg/encoding/json/encode.go File src/pkg/encoding/json/encode.go (right): https://codereview.appspot.com/6997045/diff/5001/src/pkg/encoding/json/encode.go#newcode432 src/pkg/encoding/json/encode.go:432: func isValidString(s string) bool { I don't understand what ...
9 years, 6 months ago (2012-12-22 10:14:06 UTC) #4
DMorsing
This CL doesn't seem to handle the case where someone wants a , in their ...
9 years, 6 months ago (2012-12-22 10:21:56 UTC) #5
rsc
Thank you for working on this. I don't believe that we need to go quite ...
9 years, 6 months ago (2012-12-22 15:40:28 UTC) #6
stephane.travostino
On 2012/12/22 10:14:06, remyoudompheng wrote: > https://codereview.appspot.com/6997045/diff/5001/src/pkg/encoding/json/encode.go > File src/pkg/encoding/json/encode.go (right): > > https://codereview.appspot.com/6997045/diff/5001/src/pkg/encoding/json/encode.go#newcode432 > ...
9 years, 6 months ago (2012-12-22 16:17:53 UTC) #7
rsc
The json keys we allow in struct tags are a subset of those allowed by ...
9 years, 6 months ago (2012-12-22 16:19:48 UTC) #8
stephane.travostino
@rsc, @DMorsing: I know that corner cases such as "," being a valid JSON string ...
9 years, 6 months ago (2012-12-22 16:21:54 UTC) #9
rsc
We are intentionally reserving some syntax for future expansion. We will gladly consider specific characters ...
9 years, 6 months ago (2012-12-22 16:32:30 UTC) #10
stephane.travostino
PTAL
9 years, 6 months ago (2012-12-22 16:50:16 UTC) #11
rsc
LGTM Thanks. The backwards compatibility promise means we have to be very conservative about what ...
9 years, 6 months ago (2012-12-22 18:35:58 UTC) #12
rsc
9 years, 6 months ago (2012-12-22 18:37:09 UTC) #13
*** Submitted as https://code.google.com/p/go/source/detail?r=bd8499d19a94 ***

encoding/json: A JSON tag can be any valid JSON string.

Fixes issue 3887.

R=golang-dev, daniel.morsing, remyoudompheng, rsc
CC=golang-dev
https://codereview.appspot.com/6997045

Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.

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