code review 12541051: encoding: new package

encoding: new package

See golang.org/s/go12encoding for design.

[rsc, 2013-08-09 18:00:35]

Hello r (cc: golang-dev@googlegroups.com),

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

[r, 2013-08-10 21:24:47]

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.go
File src/pkg/encoding/encoding.go (right):

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.g...
src/pkg/encoding/encoding.go:6: // multiple encoding implementations.
this doesn't say much and encoding, although a fine package name, is pretty
sweeping. what this does is more strictly defined.

perhaps something like

Package encoding defines interfaces shared by other packages that convert data
to and from byte-level and textual representations. Packages that implement
these interfaces include encoding/json, ... (pick some good ones and include at
least one unusual one). The interfaces come in pairs that, respectively, produce
and consume encoded data.

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.g...
src/pkg/encoding/encoding.go:20: // UnmarshalBinary should be able to decode the
form generated by MarshalBinary.
s/should/must/ or rewrite to make it clear these are duals.
same below.

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.g...
src/pkg/encoding/encoding.go:31: // MarshalText encodes the receiver into UTF-8
text and returns the result.
UTF-8-encoded text

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.g...
src/pkg/encoding/encoding.go:33: // MarshalText is expected to return a
complete, preferably human-readable description
this is a very odd sentence that requires careful reading to understand. i'm not
certain i do understand it. can you explain this more clearly? the "preferably"
is wrong, i think. Stringer makes no claim of preference.

[rsc, 2013-08-13 13:22:54]

PTAL

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.go
File src/pkg/encoding/encoding.go (right):

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.g...
src/pkg/encoding/encoding.go:6: // multiple encoding implementations.
On 2013/08/10 21:24:47, r wrote:
> this doesn't say much and encoding, although a fine package name, is pretty
> sweeping. what this does is more strictly defined.
> 
> perhaps something like
> 
> Package encoding defines interfaces shared by other packages that convert data
> to and from byte-level and textual representations. Packages that implement
> these interfaces include encoding/json, ... (pick some good ones and include
at
> least one unusual one). The interfaces come in pairs that, respectively,
produce
> and consume encoded data.
> 
> 

Done.

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.g...
src/pkg/encoding/encoding.go:20: // UnmarshalBinary should be able to decode the
form generated by MarshalBinary.
On 2013/08/10 21:24:47, r wrote:
> s/should/must/ or rewrite to make it clear these are duals.
> same below.

Done.

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.g...
src/pkg/encoding/encoding.go:31: // MarshalText encodes the receiver into UTF-8
text and returns the result.
On 2013/08/10 21:24:47, r wrote:
> UTF-8-encoded text

Done.

https://codereview.appspot.com/12541051/diff/6001/src/pkg/encoding/encoding.g...
src/pkg/encoding/encoding.go:33: // MarshalText is expected to return a
complete, preferably human-readable description
On 2013/08/10 21:24:47, r wrote:
> this is a very odd sentence that requires careful reading to understand. i'm
not
> certain i do understand it. can you explain this more clearly? the
"preferably"
> is wrong, i think. Stringer makes no claim of preference.

Yes, it was very hard to write. I was trying to explain why TextMarshaler and
fmt.Stringer are different. But maybe it's best not to bother. Deleted.

[r, 2013-08-13 22:27:12]

LGTM

https://codereview.appspot.com/12541051/diff/12001/src/pkg/encoding/encoding.go
File src/pkg/encoding/encoding.go (right):

https://codereview.appspot.com/12541051/diff/12001/src/pkg/encoding/encoding....
src/pkg/encoding/encoding.go:26: //
d

https://codereview.appspot.com/12541051/diff/12001/src/pkg/encoding/encoding....
src/pkg/encoding/encoding.go:45: //
d

[rsc, 2013-08-14 04:18:24]

*** Submitted as https://code.google.com/p/go/source/detail?r=00b5a40ae411 ***

encoding: new package

See golang.org/s/go12encoding for design.

R=r
CC=golang-dev
https://codereview.appspot.com/12541051