code review 5434095: strconv: new API

strconv: new API

[rsc, 2011-12-01 22:43:31]

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

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-12-01 22:44:46]

I sent these out mainly for feedback on the API.
I am not proposing to check these in until next week.
There seems to be enough in the air right now.

Russ

[bradfitz, 2011-12-01 22:57:06]

Much nicer.

On Thu, Dec 1, 2011 at 2:44 PM, Russ Cox <rsc@golang.org> wrote:

> I sent these out mainly for feedback on the API.
> I am not proposing to check these in until next week.
> There seems to be enough in the air right now.
>
> Russ
>

[bradfitz, 2011-12-01 23:05:31]

http://codereview.appspot.com/5434095/diff/1002/src/pkg/strconv/ftoa.go
File src/pkg/strconv/ftoa.go (right):

http://codereview.appspot.com/5434095/diff/1002/src/pkg/strconv/ftoa.go#newco...
src/pkg/strconv/ftoa.go:45: // Ftoa32(f) is not the same as Ftoa64(float32(f)),
stale comment

[gri, 2011-12-01 23:11:20]

LGTM

http://codereview.appspot.com/5434095/diff/1002/src/pkg/strconv/atof.go
File src/pkg/strconv/atof.go (right):

http://codereview.appspot.com/5434095/diff/1002/src/pkg/strconv/atof.go#newco...
src/pkg/strconv/atof.go:407: f1, err1 := atof64(s)
just

return atof64(s)

[r, 2011-12-02 00:17:13]

LGTM
much happier

[bradfitz, 2011-12-05 17:33:08]

On Thu, Dec 1, 2011 at 2:44 PM, Russ Cox <rsc@golang.org> wrote:

> I sent these out mainly for feedback on the API.
> I am not proposing to check these in until next week.


I'll redo the json stuff to use these, once this goes in.

[rsc, 2011-12-05 18:46:02]

On Mon, Dec 5, 2011 at 12:33, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> I'll redo the json stuff to use these, once this goes in.

thanks.  should probably wait until the fast version goes in.  :-)

[agl1, 2011-12-05 18:51:50]

http://codereview.appspot.com/5434095/diff/1002/src/pkg/strconv/atoi.go
File src/pkg/strconv/atoi.go (right):

http://codereview.appspot.com/5434095/diff/1002/src/pkg/strconv/atoi.go#newco...
src/pkg/strconv/atoi.go:134: // to s cannot be represented by an unsigned
integer of the
s/unsigned/signed/ ? You appear to be doing |bitSize-1| below, which suggests
that you're checking whether it fits into a signed number of the given size.

[bradfitz, 2011-12-05 18:57:39]

On Mon, Dec 5, 2011 at 10:46 AM, Russ Cox <rsc@golang.org> wrote:

> On Mon, Dec 5, 2011 at 12:33, Brad Fitzpatrick <bradfitz@golang.org>
> wrote:
> > I'll redo the json stuff to use these, once this goes in.
>
> thanks.  should probably wait until the fast version goes in.  :-)
>

I can make the one call I need fast in the same CL.  The others can go in
incrementally.

[rsc, 2011-12-05 20:05:21]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, gri@golang.org,
r@golang.org, agl@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-12-05 20:06:21]

PTAL

Added tests and fixed comment bug pointed out by agl.

[bradfitz, 2011-12-05 20:17:47]

LGTM

On Mon, Dec 5, 2011 at 12:06 PM, Russ Cox <rsc@golang.org> wrote:

> PTAL
>
> Added tests and fixed comment bug pointed out by agl.
>

[gri, 2011-12-05 20:39:28]

LGTM

http://codereview.appspot.com/5434095/diff/1006/src/pkg/strconv/atof.go
File src/pkg/strconv/atof.go (right):

http://codereview.appspot.com/5434095/diff/1006/src/pkg/strconv/atof.go#newco...
src/pkg/strconv/atof.go:407: f1, err1 := atof64(s)
I guess you decided against simply

return atof64(s)

here?

[rsc, 2011-12-05 20:43:49]

On Mon, Dec 5, 2011 at 15:39,  <gri@golang.org> wrote:
> I guess you decided against simply
>
> return atof64(s)
>
> here?

sorry, forgot to apply that comment.  done.
it was a dreg from an earlier time.

[rsc, 2011-12-05 20:48:25]

*** Submitted as http://code.google.com/p/go/source/detail?r=497c07d73643 ***

strconv: new API

R=golang-dev, bradfitz, gri, r, agl
CC=golang-dev
http://codereview.appspot.com/5434095