code review 5693060: encoding/gob: fix "// +build" comment for debug.go

encoding/gob: fix "// +build" comment for debug.go

[minux1, 2012-02-24 19:39:55]

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

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

[minux1, 2012-02-24 19:40:31]

I wonder if this fact should be documented in go/build/dir.go.

[rsc1, 2012-02-24 20:11:36]

How about just // +build ignore?

[minux1, 2012-02-25 19:03:53]

On 2012/02/24 20:11:36, rsc1 wrote:
> How about just // +build ignore?
It was originally "// +build ignore" in b479bc080ad9, and changed to the current
one
in c1799a84a1d4.

IMO, this file isn't the same as other ignored files. A good compromise would be
// +build debug
But I'm afraid the "debug" build tag is too common for people to (mis)use.

Another question, will this (the build tag and gob.Debug) be considered as
public interface?

[adg, 2012-02-27 03:30:08]

On 26 February 2012 06:03,  <minux.ma@gmail.com> wrote:
> On 2012/02/24 20:11:36, rsc1 wrote:
>>
>> How about just // +build ignore?
>
> It was originally "// +build ignore" in b479bc080ad9, and changed to the
> current one
> in c1799a84a1d4.
>
> IMO, this file isn't the same as other ignored files.

In what way? "ignore" seems the perfect tag to use here. It is
well-defined and does exactly what you want.

> A good compromise
> would be
> // +build debug
> But I'm afraid the "debug" build tag is too common for people to
> (mis)use.
>
> Another question, will this (the build tag and gob.Debug) be considered
> as public interface?
>
> http://codereview.appspot.com/5693060/

[r2, 2012-02-27 04:10:51]

// +build ignore
would be fine.

-rob

[minux1, 2012-02-27 12:19:47]

On Mon, Feb 27, 2012 at 11:29 AM, Andrew Gerrand <adg@golang.org> wrote:

> On 26 February 2012 06:03,  <minux.ma@gmail.com> wrote:
> > On 2012/02/24 20:11:36, rsc1 wrote:
> >>
> >> How about just // +build ignore?
> >
> > It was originally "// +build ignore" in b479bc080ad9, and changed to the
> > current one
> > in c1799a84a1d4.
> >
> > IMO, this file isn't the same as other ignored files.
>
> In what way? "ignore" seems the perfect tag to use here. It is
> well-defined and does exactly what you want.

The reason is:
when we do 'go build -t sometag -o dump dump.go', the sometag will be
applied globally,
that is, if we change to '// +build ignore' in debug.go, then we won't be
able to build dump.go
without deleting the "// +build ignore" line in debug.go.

I think this is the reason Russ has changed it from ignore to gob-debug in
the first place
(
http://codereview.appspot.com/5601057/diff/5176/src/pkg/encoding/gob/debug.go
 ).

[rsc1, 2012-02-27 16:01:47]

The comment above asks for a deletion, so I think // +build ignore is fine.

[minux1, 2012-02-27 16:54:38]

PTAL.

Because we are using ignore as build tag, we can no longer build dump.go without
modifying debug.go, so I removed the Makefile.

[r, 2012-02-27 20:53:49]

http://codereview.appspot.com/5693060/diff/6003/src/pkg/encoding/gob/debug.go
File src/pkg/encoding/gob/debug.go (right):

http://codereview.appspot.com/5693060/diff/6003/src/pkg/encoding/gob/debug.go...
src/pkg/encoding/gob/debug.go:11: // Install gob after delete the above "//
+build ignore" line to add Debug to the gob package.
// To enable the Debug function, delete the +build ignore line above and do
//<tab>go install

and put that after the sentence on lines 12 and 13

[minux1, 2012-02-28 05:34:16]

PTAL.

On 2012/02/27 20:53:49, r wrote:
> http://codereview.appspot.com/5693060/diff/6003/src/pkg/encoding/gob/debug.go
> File src/pkg/encoding/gob/debug.go (right):
> 
>
http://codereview.appspot.com/5693060/diff/6003/src/pkg/encoding/gob/debug.go...
> src/pkg/encoding/gob/debug.go:11: // Install gob after delete the above "//
> +build ignore" line to add Debug to the gob package.
> // To enable the Debug function, delete the +build ignore line above and do
> //<tab>go install
> 
> and put that after the sentence on lines 12 and 13
Done.

[minux1, 2012-02-28 05:34:16]

PTAL.

On 2012/02/27 20:53:49, r wrote:
> http://codereview.appspot.com/5693060/diff/6003/src/pkg/encoding/gob/debug.go
> File src/pkg/encoding/gob/debug.go (right):
> 
>
http://codereview.appspot.com/5693060/diff/6003/src/pkg/encoding/gob/debug.go...
> src/pkg/encoding/gob/debug.go:11: // Install gob after delete the above "//
> +build ignore" line to add Debug to the gob package.
> // To enable the Debug function, delete the +build ignore line above and do
> //<tab>go install
> 
> and put that after the sentence on lines 12 and 13
Done.

[r, 2012-02-28 05:35:33]

LGTM

[minux1, 2012-02-28 05:39:53]

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

encoding/gob: fix "// +build" comment for debug.go

R=golang-dev, rsc, adg, r, r
CC=golang-dev
http://codereview.appspot.com/5693060