Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/godoc: examples should skip build tag annotations in -play mode #26490

Closed
mostynb opened this issue Jul 19, 2018 · 7 comments
Closed

x/tools/godoc: examples should skip build tag annotations in -play mode #26490

mostynb opened this issue Jul 19, 2018 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@mostynb
Copy link
Contributor

mostynb commented Jul 19, 2018

What did you do?

After moving the filepath.Walk example to a standalone example file (so it could use a standalone function), godoc includes the build tag annotation ("// +build !windows,!plan9" in this case) in the runnable example.

What did you expect to see?

The example, without the build tag annotation.
@gopherbot gopherbot added this to the Unreleased milestone Jul 19, 2018
mostynb added a commit to mostynb/tools that referenced this issue Jul 19, 2018
@mostynb
go.tools/godoc: also ignore build tag annotations
7fc473b 
After moving the filepath.Walk example to a standalone example file[*]
(so it could use a standalone function), godoc includes the build
tag annotation ("// +build !windows,!plan9" in this case) in the
runnable example.  The example runs correctly, but the annotation
might be confusing for new users.

I think godoc should skip these annotations.

Fixes golang/go#26490.

[*] https://go-review.googlesource.com/c/go/+/122237
@bcmills
Copy link
Contributor

bcmills commented Jul 20, 2018

CC @andybons @agnivade @bradfitz @griesemer @ysmolsky
(godoc has many owners!)

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 20, 2018
@bradfitz bradfitz changed the title x/tools/godoc should skip build tag annotations x/tools/godoc: should skip build tag annotations Jul 20, 2018
@bradfitz
Copy link
Contributor

So you just want to remove the +build lines from the output, and only for examples? Where?

I don't see any +build lines at https://tip.golang.org/pkg/path/filepath/#Walk

@agnivade
Copy link
Contributor

I moved the ExampleWalk function to a separate file my_test.go with the same build tags // +build !windows,!plan9. But I could not repro your issue. I am on linux btw, if it matters.

Could you mention the steps that you are doing to get this ?

@agnivade agnivade added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 20, 2018
@agnivade
Copy link
Contributor

agnivade commented Jul 20, 2018 •
edited

Ah nvm, I wasn't on latest master. I see it now.

EDIT: @bradfitz the issue only occurs when the examples are runnable. tip.golang.org is not running with -play flag. You can repro the issue locally.

@agnivade agnivade removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 20, 2018
@agnivade agnivade changed the title x/tools/godoc: should skip build tag annotations x/tools/godoc: examples should skip build tag annotations in -play mode Jul 20, 2018
@agnivade agnivade added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 20, 2018
mostynb added a commit to mostynb/tools that referenced this issue Jul 20, 2018
@mostynb
godoc: also ignore build tag annotations
0e5b3bd 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example.  The example runs correctly, but the
annotation might be confusing for new users.

I think godoc should skip these annotations, when displaying examples.

Fixes golang/go#26490.
@gopherbot
Copy link

Change https://golang.org/cl/125040 mentions this issue: godoc: also ignore build tag annotations

@mostynb mostynb mentioned this issue Jul 20, 2018
Closed
mostynb added a commit to mostynb/tools that referenced this issue Jul 20, 2018
@mostynb
godoc: skip build tag annotations when displaying examples
03f81e0 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example.  The example runs correctly, but the
annotation might be confusing for new users.

I think godoc should skip these annotations when displaying examples.

Fixes golang/go#26490.

Change-Id: Ia4a542a48f28b03661ec881229124c7c7bf44646
mostynb added a commit to mostynb/tools that referenced this issue Jul 20, 2018
@mostynb
godoc: skip build tag annotations when displaying examples
16832f6 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example.  The example runs correctly, but the
annotation might be confusing for new users.

I think godoc should skip these annotations when displaying examples.

Fixes golang/go#26490.

Change-Id: Ia4a542a48f28b03661ec881229124c7c7bf44646
mostynb added a commit to mostynb/tools that referenced this issue Jul 20, 2018
@mostynb
godoc: skip build tag annotations when displaying examples
b96bb18 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example.  The example runs correctly, but the
annotation might be confusing for new users.

I think godoc should skip these annotations when displaying examples.

Fixes golang/go#26490.

Change-Id: Ia4a542a48f28b03661ec881229124c7c7bf44646
mostynb added a commit to mostynb/tools that referenced this issue Jul 20, 2018
@mostynb
godoc: skip build tag annotations when displaying examples
52beabd 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example.  The example runs correctly, but the
annotation might be confusing for new users.

I think godoc should skip these annotations when displaying examples.

Fixes golang/go#26490.

Change-Id: Ia4a542a48f28b03661ec881229124c7c7bf44646
@mostynb
Copy link
Contributor Author

mostynb commented Jul 20, 2018

Sorry for not mentioning the testcase earlier. On the tip of master, run
godoc -http=:6060 -play=true

Then open http://localhost:6060/pkg/path/filepath/#Walk in your browser and expand the Walk example: it starts with "// +build !windows,!plan9", which is not useful in the playground.

mostynb added a commit to mostynb/tools that referenced this issue Jul 26, 2018
@mostynb
Reland "godoc: skip build tag annotations when displaying examples"
08c9308 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example.  The example runs correctly, but the
annotation might be confusing for new users.

I think godoc should skip these annotations when displaying examples.

Fixes golang/go#26490.

Skipped the testcase for now, since it triggers a false positive:
golang/go#26627

Change-Id: I1da4b3b7e1e5a85a76773e25d9355b3f92479c19
@mostynb mostynb mentioned this issue Jul 26, 2018
Closed
@gopherbot
Copy link

Change https://golang.org/cl/126256 mentions this issue: Reland "godoc: skip build tag annotations when displaying examples"

mostynb added a commit to mostynb/tools that referenced this issue Jul 28, 2018
@mostynb
godoc: skip build tag annotations when displaying examples
4316b95 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example.  The example runs correctly, but the
annotation might be confusing for new users.

I think godoc should skip these annotations when displaying examples.

Fixes golang/go#26490.

Avoid using a multiline string in the test, to avoid false positives
from older versions of "go vet", which are still used on the build
dashboard.

Change-Id: I1da4b3b7e1e5a85a76773e25d9355b3f92479c19
mostynb added a commit to mostynb/tools that referenced this issue Jul 30, 2018
@mostynb
godoc: skip build tag annotations when displaying examples
6ba66c1 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example. The example runs correctly, but the
annotation might be confusing for new users.

Change the behavior so that godoc skips these annotations when
displaying examples.

To avoid false positives in older versions of "go vet", which are still used
on the build dashboard, we avoid using a multiline string in the test.

Fixes golang/go#26490.

Change-Id: I1da4b3b7e1e5a85a76773e25d9355b3f92479c19
mostynb added a commit to mostynb/tools that referenced this issue Jul 30, 2018
@mostynb
godoc: skip build tag annotations when displaying examples
b81705a 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example. The example runs correctly, but the
annotation might be confusing for new users.

Change the behavior so that godoc skips these annotations when
displaying examples.

To avoid false positives in older versions of "go vet", which are still used
on the build dashboard, we avoid using a multiline string in the test.

Fixes golang/go#26490.

Change-Id: I1da4b3b7e1e5a85a76773e25d9355b3f92479c19
mostynb added a commit to mostynb/tools that referenced this issue Jul 31, 2018
@mostynb
godoc: skip build tag annotations when displaying examples
bc5ed29 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example. The example runs correctly, but the
annotation might be confusing for new users.

Change the behavior so that godoc skips these annotations when
displaying examples.

To avoid false positives in older versions of "go vet", which are still used
on the build dashboard, we avoid using a multiline string in the test.

Fixes golang/go#26490.

Change-Id: I1da4b3b7e1e5a85a76773e25d9355b3f92479c19
gopherbot pushed a commit to golang/tools that referenced this issue Jul 31, 2018
@mostynb @andybons
godoc: skip build tag annotations when displaying examples
9e9f7f6 
After moving the filepath.Walk example to a standalone example file
in CL 122237 (so it could use a standalone function), godoc includes
the build tag annotation ("// +build !windows,!plan9" in this case)
in the runnable example.  The example runs correctly, but the
annotation might be confusing for new users.

Change the behavior so that godoc skips these annotations when
displaying examples.

To avoid false positives in older versions of "go vet", which are still used
on the build dashboard, we avoid using a multiline string in the test.

Fixes golang/go#26490.

Change-Id: I1da4b3b7e1e5a85a76773e25d9355b3f92479c19
GitHub-Last-Rev: bc5ed29
GitHub-Pull-Request: #42
Reviewed-on: https://go-review.googlesource.com/126256
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
@golang golang locked and limited conversation to collaborators Jul 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@bradfitz @agnivade @bcmills @mostynb @gopherbot