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

Issue 953041: code review 953041: json: Marshal, Unmarshal using new scanner (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 11 months ago by rsc
Modified:
14 years, 10 months ago
Reviewers:
cw
CC:
r, rsc
Visibility:
Public.

Description

json: Marshal, Unmarshal using new scanner

Patch Set 1 #

Patch Set 2 : code review 953041: json: Marshal, Unmarshal using new scanner #

Total comments: 9

Patch Set 3 : code review 953041: json: Marshal, Unmarshal using new scanner #

Patch Set 4 : code review 953041: json: Marshal, Unmarshal using new scanner #

Patch Set 5 : code review 953041: json: Marshal, Unmarshal using new scanner #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+1574 lines, -1057 lines) Patch
M src/pkg/expvar/expvar.go View 1 chunk +4 lines, -7 lines 0 comments Download
M src/pkg/expvar/expvar_test.go View 1 chunk +2 lines, -1 line 0 comments Download
M src/pkg/json/Makefile View 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/json/decode.go View 1 2 3 2 chunks +867 lines, -75 lines 1 comment Download
M src/pkg/json/decode_test.go View 1 2 3 1 chunk +406 lines, -112 lines 0 comments Download
A src/pkg/json/encode.go View 1 2 3 1 chunk +282 lines, -0 lines 1 comment Download
M src/pkg/json/scanner.go View 2 chunks +5 lines, -1 line 0 comments Download
M src/pkg/json/scanner_test.go View 1 2 3 4 2 chunks +6 lines, -3 lines 0 comments Download
R src/pkg/json/struct.go View 1 chunk +0 lines, -481 lines 0 comments Download
R src/pkg/json/struct_test.go View 1 chunk +0 lines, -375 lines 0 comments Download
M src/pkg/template/template_test.go View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 13
rsc
Hello r (cc: golang-dev@googlegroups.com), I'd like you to review this change.
14 years, 11 months ago (2010-04-21 00:32:28 UTC) #1
r
http://codereview.appspot.com/953041/diff/2001/3004 File src/pkg/json/decode.go (right): http://codereview.appspot.com/953041/diff/2001/3004#newcode103 src/pkg/json/decode.go:103: if _, ok := r.(runtime.Error); ok { encode's error ...
14 years, 11 months ago (2010-04-21 00:53:37 UTC) #2
rsc
Done except > http://codereview.appspot.com/953041/diff/2001/3005#newcode118 > src/pkg/json/decode_test.go:118: Int64 int64 > there should be a tag in ...
14 years, 11 months ago (2010-04-21 23:15:33 UTC) #3
r2
On Apr 21, 2010, at 4:15 PM, Russ Cox wrote: > Done except > >> ...
14 years, 11 months ago (2010-04-21 23:21:15 UTC) #4
rsc
PTAL Added the code for tag handling instead.
14 years, 11 months ago (2010-04-21 23:25:52 UTC) #5
r
LGTM
14 years, 11 months ago (2010-04-21 23:29:14 UTC) #6
rsc
*** Submitted as http://code.google.com/p/go/source/detail?r=500ccf4f55f5 *** json: Marshal, Unmarshal using new scanner R=r CC=golang-dev http://codereview.appspot.com/953041
14 years, 11 months ago (2010-04-21 23:40:57 UTC) #7
cw
nag http://codereview.appspot.com/953041/diff/12002/22006 File src/pkg/json/encode.go (right): http://codereview.appspot.com/953041/diff/12002/22006#newcode57 src/pkg/json/encode.go:57: func Marshal(v interface{}) ([]byte, os.Error) { before this ...
14 years, 11 months ago (2010-04-22 08:38:34 UTC) #8
rsc
yes, that's what it means. it also means that if the code encounters an error ...
14 years, 11 months ago (2010-04-22 08:41:31 UTC) #9
cw
On Thu, Apr 22, 2010 at 01:41:26AM -0700, Russ Cox wrote: > yes, that's what ...
14 years, 11 months ago (2010-04-22 09:02:53 UTC) #10
cw
i'm wondering if we can't have a common interface for json and gobs (and potentially ...
14 years, 10 months ago (2010-04-27 18:41:22 UTC) #11
rsc
> But is there not an opportunity to have a common Unmarshaler interface > for ...
14 years, 10 months ago (2010-04-27 18:51:32 UTC) #12
cw
14 years, 10 months ago (2010-04-27 18:56:30 UTC) #13
On Tue, Apr 27, 2010 at 11:51:28AM -0700, Russ Cox wrote:

> When the json unmarshaller does a dynamic interface check
> to see if the object knows how to json unmarshal itself, it
> had better not use the gob unmarshaller instead.

ok, that makes perfect sense, i was thinking about it from the wrong
end

thanks!
Sign in to reply to this message.

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