code review 3302042: Removed bytes.Add and bytes.AddByte; we now have 'append'.

Removed bytes.Add and bytes.AddByte; we now have 'append'.
Changed all uses of bytes.Add (aside from those testing bytes.Add) to append(a, b...).
Also ran "gofmt -s" and made use of copy([]byte, string) in the fasta benchmark.

[Kyle C, 2010-12-01 05:41:59]

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

I'd like you to review this change.

[r, 2010-12-01 18:13:57]

http://codereview.appspot.com/3302042/diff/5001/src/pkg/bytes/bytes.go
File src/pkg/bytes/bytes.go (right):

http://codereview.appspot.com/3302042/diff/5001/src/pkg/bytes/bytes.go#newcod...
src/pkg/bytes/bytes.go:558: func Add(s, t []byte) []byte { // TODO
you should just delete these functions

[Kyle C, 2010-12-01 18:30:31]

http://codereview.appspot.com/3302042/diff/5001/src/pkg/bytes/bytes.go
File src/pkg/bytes/bytes.go (right):

http://codereview.appspot.com/3302042/diff/5001/src/pkg/bytes/bytes.go#newcod...
src/pkg/bytes/bytes.go:558: func Add(s, t []byte) []byte { // TODO
On 2010/12/01 18:13:57, r wrote:
> you should just delete these functions

I'd be happy to. I didn't because it would break a fair amount of pre-existing
code (including the protocol buffers, I believe), and that's not the kind of
decision I get to make.
But if it's okay with you, I'll remove these and the associated tests also.

[r2, 2010-12-01 18:31:16]

On Dec 1, 2010, at 10:30 AM, consalus@gmail.com wrote:

> 
> http://codereview.appspot.com/3302042/diff/5001/src/pkg/bytes/bytes.go
> File src/pkg/bytes/bytes.go (right):
> 
>
http://codereview.appspot.com/3302042/diff/5001/src/pkg/bytes/bytes.go#newcod...
> src/pkg/bytes/bytes.go:558: func Add(s, t []byte) []byte { // TODO
> On 2010/12/01 18:13:57, r wrote:
>> you should just delete these functions
> 
> I'd be happy to. I didn't because it would break a fair amount of
> pre-existing code (including the protocol buffers, I believe), and
> that's not the kind of decision I get to make.
> But if it's okay with you, I'll remove these and the associated tests
> also.

You have my blessing and thanks - this was on my TODO list.

-rob

[Kyle C, 2010-12-01 18:51:47]

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

Please take another look.

[r, 2010-12-01 18:54:27]

LGTM

[r2, 2010-12-01 18:56:15]

looks like you didn't run all.bash. please run that, update the cl, and let me
know.

-rob

[Kyle C, 2010-12-01 19:14:53]

Hm. Thought I ran it twice. I'll run it once again for good measure.

On 2010/12/01 18:56:15, r2 wrote:
> looks like you didn't run all.bash. please run that, update the cl, and let me
> know.
> 
> -rob
>

[Kyle C, 2010-12-01 19:15:09]

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

Please take another look.

[r2, 2010-12-01 19:22:30]

On Dec 1, 2010, at 11:14 AM, consalus@gmail.com wrote:

> Hm. Thought I ran it twice. I'll run it once again for good measure.
> 
> On 2010/12/01 18:56:15, r2 wrote:
>> looks like you didn't run all.bash. please run that, update the cl,
> and let me
>> know.
> 
>> -rob
> 
> 
> 
> 
> http://codereview.appspot.com/3302042/

i applied the patch again and i see these:

tubenose=% g AddByte */*.go
bytes/bytes_test.go:599: func TestAddByte(t *testing.T) {
bytes/bytes_test.go:603: 		b = AddByte(b, byte(i))
bytes/bytes_test.go:606: 		t.Errorf("AddByte: too small; expected %d got %d", N,
len(b))
bytes/bytes_test.go:610: 			t.Fatalf("AddByte: b[%d] should be %d is %d", i, c,
byte(i))
tubenose=% 

moreover, i think there are some in the command directory. if all.bash runs for
you, i believe you must have outstanding changes in your client not in this CL.

-rob

[Kyle C, 2010-12-01 19:51:49]

Bah. You're right. Sorry. Forgot to add "bytes_test.go".
Fixed, uploaded.

I also updated effective_go.html to use "append" instead of "bytes.Add".
Now nothing in the Go tree matches "bytes.Add" according to my grepping.

On Wed, Dec 1, 2010 at 11:22 AM, Rob 'Commander' Pike <r@google.com> wrote:

>
> On Dec 1, 2010, at 11:14 AM, consalus@gmail.com wrote:
>
> > Hm. Thought I ran it twice. I'll run it once again for good measure.
> >
> > On 2010/12/01 18:56:15, r2 wrote:
> >> looks like you didn't run all.bash. please run that, update the cl,
> > and let me
> >> know.
> >
> >> -rob
> >
> >
> >
> >
> > http://codereview.appspot.com/3302042/
>
> i applied the patch again and i see these:
>
> tubenose=% g AddByte */*.go
> bytes/bytes_test.go:599: func TestAddByte(t *testing.T) {
> bytes/bytes_test.go:603:                b = AddByte(b, byte(i))
> bytes/bytes_test.go:606:                t.Errorf("AddByte: too small;
> expected %d got %d", N, len(b))
> bytes/bytes_test.go:610:                        t.Fatalf("AddByte: b[%d]
> should be %d is %d", i, c, byte(i))
> tubenose=%
>
> moreover, i think there are some in the command directory. if all.bash runs
> for you, i believe you must have outstanding changes in your client not in
> this CL.
>
> -rob
>
>

[r, 2010-12-01 19:59:20]

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

Removed bytes.Add and bytes.AddByte; we now have 'append'.
Changed all uses of bytes.Add (aside from those testing bytes.Add) to append(a,
b...).
Also ran "gofmt -s" and made use of copy([]byte, string) in the fasta benchmark.

R=golang-dev, r, r2
CC=golang-dev
http://codereview.appspot.com/3302042

Committer: Rob Pike <r@golang.org>