Issue 8617044: code review 8617044: encoding/json: different decision on tags and shadowing

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 8617044: code review 8617044: encoding/json: different decision on tags and shadowing (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
11 years ago by r

Modified:
11 years ago

Reviewers:
rog

CC:
golang-dev, iant

Visibility:
Public.

Description

encoding/json: different decision on tags and shadowing If there are no tags, the rules are the same as before. If there is a tagged field, choose it if there is exactly one at the top level of all fields. More tests. The old tests were clearly inadequate, since they all pass as is. The new tests only work with the new code.

Patch Set 1 #

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

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

Created: 11 years ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+81 lines, -19 lines)			Patch
	M	src/pkg/encoding/json/encode.go	View	1	2 chunks	+25 lines, -17 lines	0 comments	Download
	M	src/pkg/encoding/json/encode_test.go	View		3 chunks	+56 lines, -2 lines	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/

11 years ago (2013-04-10 19:12:12 UTC) #1

iant

LGTM This seems like a reasonable semantic, and the code looks fine. Can we improve ...

11 years ago (2013-04-10 19:22:38 UTC) #2

I will work on docs once we decide on the design. Does anyone else here ...

11 years ago (2013-04-10 19:24:00 UTC) #3

*** Submitted as https://code.google.com/p/go/source/detail?r=848d7e249632 *** encoding/json: different decision on tags and shadowing If there are ...

11 years ago (2013-04-10 20:05:37 UTC) #4

rog

11 years ago (2013-04-11 09:35:14 UTC) #5

Message was sent while issue was closed.

On 2013/04/10 20:05:37, r wrote:
> *** Submitted as https://code.google.com/p/go/source/detail?r=848d7e249632 ***
> 
> encoding/json: different decision on tags and shadowing
> If there are no tags, the rules are the same as before.
> If there is a tagged field, choose it if there is exactly one
> at the top level of all fields.
> More tests. The old tests were clearly inadequate, since
> they all pass as is. The new tests only work with the new code.
> 
> R=golang-dev, iant
> CC=golang-dev
> https://codereview.appspot.com/8617044

LGTM, thanks.

Expand All Messages | Collapse All Messages