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

Issue 12556043: code review 12556043: encoding/xml: add, support Unmarshaler interface (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by rsc
Modified:
4 years, 3 months ago
CC:
golang-dev, Dominik Honnef, kortschak
Visibility:
Public.

Description

encoding/xml: add, support Unmarshaler interface See golang.org/s/go12xml for design.

Patch Set 1 #

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

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

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

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

Total comments: 3

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

Total comments: 2

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

Total comments: 1

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+313 lines, -22 lines) Patch
M src/pkg/encoding/xml/read.go View 1 2 3 4 5 4 chunks +117 lines, -5 lines 0 comments Download
M src/pkg/encoding/xml/read_test.go View 1 2 3 2 chunks +64 lines, -0 lines 0 comments Download
M src/pkg/encoding/xml/xml.go View 1 2 3 10 chunks +132 lines, -17 lines 0 comments Download

Messages

Total messages: 13
rsc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
4 years, 3 months ago (2013-08-06 18:50:32 UTC) #1
rsc
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), Please take another look.
4 years, 3 months ago (2013-08-09 15:34:19 UTC) #2
Dominik Honnef
https://codereview.appspot.com/12556043/diff/13001/src/pkg/encoding/xml/read.go File src/pkg/encoding/xml/read.go (right): https://codereview.appspot.com/12556043/diff/13001/src/pkg/encoding/xml/read.go#newcode159 src/pkg/encoding/xml/read.go:159: // Unmarshaler is the interface implemented by objects that ...
4 years, 3 months ago (2013-08-09 16:21:44 UTC) #3
kortschak
https://codereview.appspot.com/12556043/diff/18001/src/pkg/encoding/xml/read.go File src/pkg/encoding/xml/read.go (right): https://codereview.appspot.com/12556043/diff/18001/src/pkg/encoding/xml/read.go#newcode214 src/pkg/encoding/xml/read.go:214: // so it's likely to be incorrect, but we ...
4 years, 3 months ago (2013-08-11 01:31:19 UTC) #4
kortschak
I just ported my ncbi/entrez package over to use on this code and it was ...
4 years, 3 months ago (2013-08-12 01:31:47 UTC) #5
rsc
https://codereview.appspot.com/12556043/diff/13001/src/pkg/encoding/xml/xml.go File src/pkg/encoding/xml/xml.go (right): https://codereview.appspot.com/12556043/diff/13001/src/pkg/encoding/xml/xml.go#newcode483 src/pkg/encoding/xml/xml.go:483: return nil, errRawToken On 2013/08/09 16:21:45, Dominik Honnef wrote: ...
4 years, 3 months ago (2013-08-13 16:24:33 UTC) #6
Dominik Honnef
LGTM Just changed some code to use the Unmarshaler interface and it works great so ...
4 years, 3 months ago (2013-08-13 17:37:48 UTC) #7
kortschak
LGTM On 14/08/2013, at 1:54 AM, "rsc@golang.org" <rsc@golang.org> wrote: > It's possible to use correctly. ...
4 years, 3 months ago (2013-08-13 21:04:49 UTC) #8
rsc
Hello golang-dev@googlegroups.com, dominik.honnef@gmail.com, dan.kortschak@adelaide.edu.au (cc: golang-dev@googlegroups.com), Please take another look.
4 years, 3 months ago (2013-08-14 04:35:33 UTC) #9
Dominik Honnef
https://codereview.appspot.com/12556043/diff/28001/src/pkg/encoding/xml/xml.go File src/pkg/encoding/xml/xml.go (right): https://codereview.appspot.com/12556043/diff/28001/src/pkg/encoding/xml/xml.go#newcode1859 src/pkg/encoding/xml/xml.go:1859: func (p *printer) EscapeString(s string) { Why is EscapeString ...
4 years, 3 months ago (2013-08-14 16:28:33 UTC) #10
rsc
On Wed, Aug 14, 2013 at 12:28 PM, <dominik.honnef@gmail.com> wrote: > > https://codereview.appspot.**com/12556043/diff/28001/src/** > pkg/encoding/xml/xml.go<https://codereview.appspot.com/12556043/diff/28001/src/pkg/encoding/xml/xml.go> ...
4 years, 3 months ago (2013-08-14 17:02:20 UTC) #11
rsc
On Tue, Aug 13, 2013 at 5:04 PM, Dan Kortschak < dan.kortschak@adelaide.edu.au> wrote: > Maybe ...
4 years, 3 months ago (2013-08-14 18:57:32 UTC) #12
rsc
4 years, 3 months ago (2013-08-14 18:58:14 UTC) #13
*** Submitted as https://code.google.com/p/go/source/detail?r=add6c198070e ***

encoding/xml: add, support Unmarshaler interface

See golang.org/s/go12xml for design.

R=golang-dev, dominik.honnef, dan.kortschak
CC=golang-dev
https://codereview.appspot.com/12556043
Sign in to reply to this message.

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