code review 12603044: encoding/xml: add, support Marshaler interface (issue 12603044)

100 views
Skip to first unread message

r...@golang.org

unread,
Aug 9, 2013, 11:34:39 AM8/9/13
to golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: golang-dev1,

Message:
Hello golan...@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/


Description:
encoding/xml: add, support Marshaler interface

See golang.org/s/go12xml for design.

Please review this at https://codereview.appspot.com/12603044/

Affected files:
M src/pkg/encoding/xml/marshal.go
M src/pkg/encoding/xml/marshal_test.go


ia...@golang.org

unread,
Aug 9, 2013, 12:02:45 PM8/9/13
to r...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/12603044/diff/5001/src/pkg/encoding/xml/marshal.go
File src/pkg/encoding/xml/marshal.go (right):

https://codereview.appspot.com/12603044/diff/5001/src/pkg/encoding/xml/marshal.go#newcode89
src/pkg/encoding/xml/marshal.go:89: // to encode it uses
e.EncodeElement.
s/uses/using/

https://codereview.appspot.com/12603044/diff/5001/src/pkg/encoding/xml/marshal.go#newcode98
src/pkg/encoding/xml/marshal.go:98: // Marshaler is the interface
implemented by objects that can marshal
s/Marshaler/MarshalerAttr/

https://codereview.appspot.com/12603044/diff/5001/src/pkg/encoding/xml/marshal.go#newcode101
src/pkg/encoding/xml/marshal.go:101: // MarshalXMLAttr encodes the
receiver as an XML attribute.
How about something more like
MarshalXMLAttr returns an XML attribute with the encoded value of the
receiver.

https://codereview.appspot.com/12603044/

Russ Cox

unread,
Aug 9, 2013, 2:35:44 PM8/9/13
to Russ Cox, golang-dev, Ian Taylor, re...@codereview-hr.appspotmail.com
Done.

r...@golang.org

unread,
Aug 13, 2013, 4:16:24 PM8/13/13
to golan...@googlegroups.com, ia...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com

dan.ko...@adelaide.edu.au

unread,
Aug 13, 2013, 7:28:40 PM8/13/13
to r...@golang.org, golan...@googlegroups.com, ia...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com

r...@golang.org

unread,
Aug 14, 2013, 12:17:46 AM8/14/13
to r...@golang.org, golan...@googlegroups.com, ia...@golang.org, dan.ko...@adelaide.edu.au, re...@codereview-hr.appspotmail.com
*** Submitted as
https://code.google.com/p/go/source/detail?r=2ca230b93195 ***

encoding/xml: add, support Marshaler interface

See golang.org/s/go12xml for design.

Fixes issue 2771.
Fixes issue 4169.
Fixes issue 5975.
Fixes issue 6125.

R=golang-dev, iant, dan.kortschak
CC=golang-dev
https://codereview.appspot.com/12603044


https://codereview.appspot.com/12603044/

Russ Cox

unread,
Aug 14, 2013, 12:19:37 AM8/14/13
to Russ Cox, golang-dev, Ian Taylor, Dan Kortschak, re...@codereview-hr.appspotmail.com
I did not intend to submit this; I pasted the wrong CL number while trying to submit the encoding CL. I'm still happy to make changes to this code in follow-up CLs.


Russ Cox

unread,
Aug 14, 2013, 12:22:28 AM8/14/13
to Russ Cox, golang-dev, Ian Taylor, Dan Kortschak, re...@codereview-hr.appspotmail.com
I rolled this CL back. The new pending copy is https://codereview.appspot.com/12919043. Please leave comments there.



Reply all
Reply to author
Forward
0 new messages