Re: code review 13532052: go.tools/cover: clarify mode is only necessary wh... (issue 13532052)

[n...@nathany.com]

On 2013/09/23 22:26:44, r wrote:
> p.s. we should make the -help change (printing help only when asked)
separately.
> i can do that if you like, or you can take a crack at it in a separate
CL.

I would be happy to take it on once this CL is ready to merge.

https://codereview.appspot.com/13532052/

[n...@nathany.com]

https://codereview.appspot.com/13532052/diff/22001/cmd/cover/cover.go
File cmd/cover/cover.go (right):

https://codereview.appspot.com/13532052/diff/22001/cmd/cover/cover.go#newcode37
cmd/cover/cover.go:37: go test -coverprofile=c.out -covermode=set
On 2013/09/23 22:25:58, r wrote:
> now i think this just makes it seem like you have to specify the
covermode when

Maybe adding -covermode=set wasn't a stroke of genius after all. This
still comes across as the only option instead of an example (other
covermodes, etc.).

https://codereview.appspot.com/13532052/

[n...@nathany.com]

https://codereview.appspot.com/13532052/diff/31001/cmd/cover/cover.go
File cmd/cover/cover.go (right):

https://codereview.appspot.com/13532052/diff/31001/cmd/cover/cover.go#newcode48
cmd/cover/cover.go:48: go tool cover -func=c.out
This doesn't provide an example or instruction for using -func with -o.
Is that fine?

https://codereview.appspot.com/13532052/diff/31001/cmd/cover/cover.go#newcode60
cmd/cover/cover.go:60: Name of coverage variable to generate
html & func are appropriately excluded from this list, and the flags are
indented to be part of this usage scenario.

a little more work when adding a Flag, but IMO better results

https://codereview.appspot.com/13532052/

[Rob Pike]

Please let me know when it's ready for another look.

-rob

[n...@nathany.com]

On 2013/09/24 02:48:17, r wrote:
> Please let me know when it's ready for another look.

> -rob

Please do. Thanks.

https://codereview.appspot.com/13532052/

[r...@golang.org]

https://codereview.appspot.com/13532052/diff/27001/cmd/cover/cover.go
File cmd/cover/cover.go (right):

https://codereview.appspot.com/13532052/diff/27001/cmd/cover/cover.go#newcode38
cmd/cover/cover.go:38: Given a coverage profile produced by 'go test':
s/:/, for example by running:

https://codereview.appspot.com/13532052/diff/27001/cmd/cover/cover.go#newcode60

cmd/cover/cover.go:60: Name of coverage variable to generate

you don't need to go through this because humans never run this. it's
for go test only.

The cover tool is used by "go test -cover" to to rewrite source code
with coverage annotations with an invocation such as
go tool cover -mode=set -var=CoverageVariableName program.go
These flags are documented separately. Such processing happens
automatically; users do not need to run the tool this way.

https://codereview.appspot.com/13532052/

[n...@nathany.com]

https://codereview.appspot.com/13532052/diff/27001/cmd/cover/cover.go
File cmd/cover/cover.go (right):

https://codereview.appspot.com/13532052/diff/27001/cmd/cover/cover.go#newcode60
cmd/cover/cover.go:60: Name of coverage variable to generate

On 2013/09/24 03:29:50, r wrote:
> you don't need to go through this because humans never run this. it's
for go
> test only.

I just wanted the flags indented as part of this section. Also splitting
the flags up into two groups, with html & func documented above and not
here.

[n...@nathany.com]

I reduced the indentation to align with the flags below, though it still
includes func & html in that section.

Perhaps a better option is to move the "source code rewriting" help
under another option, eg. go tool cover -help -v.

Then the usage information for most people could end something like
this:

See 'go help testflag' for details on producing a coverage profile.

Cover is used by 'go test -cover' to rewrite source code with coverage
annotations. See 'go tool cover -help -v' for advanced usage.

If you like that option, do you have a suggestion for the advanced usage
flag?

https://codereview.appspot.com/13532052/

[Rob Pike]

Please let's just bring this to ground in the form I suggested.

-rob

[n...@nathany.com]

On 2013/09/24 04:16:25, r wrote:
> Please let's just bring this to ground in the form I suggested.

> -rob

Okay. Thanks for your patience with me. I hope the 10th one's the charm.

---
Usage of 'go tool cover':

Given a coverage profile produced by 'go test':

go test -coverprofile=c.out

Open a web browser displaying annotated source code:
go tool cover -html=c.out

Write out an HTML file instead of launching a web browser:
go tool cover -html=c.out -o coverage.html

Display coverage percentages to stdout for each function:
go tool cover -func=c.out

Or to generate modified source code with coverage annotations (advanced
usage):

go tool cover -mode=set -var=CoverageVariableName program.go

Flags:
-func="": output coverage profile information for each function
-html="": generate HTML representation of coverage profile
-mode="": coverage mode: set, count, atomic
-o="": file for output; default: stdout
-var="GoCover": name of coverage variable to generate

Only one of -html, -func, or -mode may be set.


https://codereview.appspot.com/13532052/

[r...@golang.org]

LGTM


https://codereview.appspot.com/13532052/diff/34001/cmd/cover/cover.go
File cmd/cover/cover.go (right):

https://codereview.appspot.com/13532052/diff/34001/cmd/cover/cover.go#newcode49
cmd/cover/cover.go:49: Or to generate modified source code with coverage
annotations (advanced usage):
it's not advanced usage. it's never used usage. this is for the go tool.
please see the text i proposed last time.

the rest is looking like a genuine improvement.

https://codereview.appspot.com/13532052/

[n...@nathany.com]

Thanks. Though this could've went a lot faster if I learned to listen
better!

Can we call it done?

Finally, to generate modified source code with coverage annotations
(what go test -cover does):

go tool cover -mode=set -var=CoverageVariableName program.go

--
Tomorrow the "flag provided but not defined" bit.


https://codereview.appspot.com/13532052/

[r...@golang.org]

*** Submitted as
https://code.google.com/p/go/source/detail?r=e8594b80cffd&repo=tools ***

go.tools/cover: clean up usage information

R=golang-dev, r, minux.ma
CC=golang-dev
https://codereview.appspot.com/13532052

Committer: Rob Pike <r...@golang.org>


https://codereview.appspot.com/13532052/