code review 7065063: os: use syscall.Pipe2 on Linux

os: use syscall.Pipe2 on Linux

Update issue 2656

[guelfey, 2013-01-11 13:03:16]

Hello golang-dev@googlegroups.com,

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

[iant, 2013-01-11 14:58:19]

CL description should say exactly

Updates issue 2656.

That syntax is recognized by the code.google.com issue tracker.

Thanks for working on this.

https://codereview.appspot.com/7065063/diff/4001/src/pkg/os/pipe_linux.go
File src/pkg/os/pipe_linux.go (right):

https://codereview.appspot.com/7065063/diff/4001/src/pkg/os/pipe_linux.go#new...
src/pkg/os/pipe_linux.go:4: 
Missing // +build linux

[guelfey, 2013-01-11 15:05:11]

On 2013/01/11 14:58:19, iant wrote:
> CL description should say exactly
> 
> Updates issue 2656.
> 
> That syntax is recognized by the http://code.google.com issue tracker.
> 

Sorry, didn't know that. Fixed.

> https://codereview.appspot.com/7065063/diff/4001/src/pkg/os/pipe_linux.go
> File src/pkg/os/pipe_linux.go (right):
> 
>
https://codereview.appspot.com/7065063/diff/4001/src/pkg/os/pipe_linux.go#new...
> src/pkg/os/pipe_linux.go:4: 
> Missing // +build linux

Doesn't the build tool figure this out by the _linux suffix?

[minux1, 2013-01-11 15:07:07]

On Fri, Jan 11, 2013 at 10:58 PM, <iant@golang.org> wrote:

> CL description should say exactly
>
> Updates issue 2656.
>
Update issue 2656.

>
> That syntax is recognized by the code.google.com issue tracker.
>

[guelfey, 2013-01-11 15:18:12]

On 2013/01/11 15:07:07, minux wrote:
> On Fri, Jan 11, 2013 at 10:58 PM, <mailto:iant@golang.org> wrote:
> 
> > CL description should say exactly
> >
> > Updates issue 2656.
> >
> Update issue 2656.
> 
> >
> > That syntax is recognized by the http://code.google.com issue tracker.
> >
According to
http://code.google.com/p/support/wiki/IssueTracker#Integration_with_version_c...,
the correct syntax is "Update issue 2656" (without a full stop).

[bradfitz, 2013-01-11 15:55:46]

https://codereview.appspot.com/7065063/diff/4001/src/pkg/os/pipe_linux.go
File src/pkg/os/pipe_linux.go (right):

https://codereview.appspot.com/7065063/diff/4001/src/pkg/os/pipe_linux.go#new...
src/pkg/os/pipe_linux.go:4: 
On 2013/01/11 14:58:19, iant wrote:
> Missing // +build linux

http://golang.org/pkg/go/build/ says:

"If a file's name, after stripping the extension and a possible _test suffix,
matches *_GOOS, *_GOARCH, or *_GOOS_GOARCH for any known operating system and
architecture values, then the file is considered to have an implicit build
constraint requiring those terms."


We could still add it for explicitness, though.

[iant, 2013-01-11 16:27:36]

On 2013/01/11 15:05:11, guelfey wrote:
> 
> Doesn't the build tool figure this out by the _linux suffix?

Whoops, sorry, you're quite right.

[iant, 2013-01-11 16:27:53]

LGTM

[iant, 2013-01-11 16:30:33]

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

os: use syscall.Pipe2 on Linux

Update issue 2656

R=golang-dev, iant, minux.ma, bradfitz
CC=golang-dev
https://codereview.appspot.com/7065063

Committer: Ian Lance Taylor <iant@golang.org>