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

website: enable godoc analysis mode by default? #11251

Open
alml opened this Issue Jun 17, 2015 · 12 comments

Comments

Projects
None yet
6 participants
@alml

alml commented Jun 17, 2015

It would be useful to annotate interfaces in the godoc with list of known implementers.
Use case: I want to use text/scanner package and parse some data I receive from network. While reading its documentation I encounter a reference to io.Reader: https://golang.org/pkg/text/scanner/#Scanner.Init. I click on "io.Reader", and it would be nice to know that besides of net.TCPConn this interface is also implemented by bufio.NewReader, which can be used in my case.

Caveat: number of types implementing trivial interfaces can be huge.

@minux

This comment has been minimized.

Member

minux commented Jun 17, 2015

@bradfitz

This comment has been minimized.

Member

bradfitz commented Jun 17, 2015

I think this bug is about having godoc's analysis mode enabled by default for golang.org

Related: #8968

/cc @alandonovan @adg

@bradfitz bradfitz changed the title from "Known implementers" in godoc to website: enable godoc analysis mode by default? Jun 17, 2015

@alml

This comment has been minimized.

alml commented Jun 18, 2015

Thanks! I missed "-analysis type" indeed. This does exactly what I asked for. And yes, it would be great to have it enabled on golang.org and godoc.org.

@minux

This comment has been minimized.

Member

minux commented Jun 18, 2015

@alml

This comment has been minimized.

alml commented Jun 18, 2015

Right, I mentioned this in the first post. It's a presentation issue, and I'm pretty sure it can be solved somehow. 1. Show "too many" if there are many implementers, possibly a href leading to a separate URL with full list. 2. Show some packages (most popular?) and ellipsis in the end. 3. Paginate the list. 4. Anything more smart.

@minux

This comment has been minimized.

Member

minux commented Jun 18, 2015

@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jul 11, 2015

@agnivade

This comment has been minimized.

Member

agnivade commented Apr 26, 2018

ping @adg

@bradfitz

This comment has been minimized.

Member

bradfitz commented Apr 26, 2018

@andybons would be the owner of this now.

@bradfitz bradfitz modified the milestones: Unreleased, Go1.11 Apr 26, 2018

@bradfitz bradfitz added the NeedsFix label Apr 26, 2018

@bradfitz

This comment has been minimized.

Member

bradfitz commented Apr 26, 2018

(It seems worth doing, unless we run into performance problems on start-up or ridiculous memory usage. If either of those occur, punt to a later release or Unreleased.)

@bradfitz

This comment has been minimized.

Member

bradfitz commented Jul 24, 2018

I just tried it.

Few oddities:

  • it starts running compilations on packages it doesn't have built. I see it compiling C-based stuff locally. But that doesn't apply to golang.org where there will be no $GOPATH. So we can ignore this.
  • Why does it care about tests?
2018/07/24 18:11:38 Adding tests for archive/zip_test
2018/07/24 18:11:38 Adding tests for cmd/vendor/github.com/google/pprof/internal/symbolz
2018/07/24 18:11:38 Adding tests for vendor/golang_org/x/crypto/internal/chacha20
2018/07/24 18:11:38 Adding tests for debug/dwarf_test
2018/07/24 18:11:38 Adding tests for html/template
2018/07/24 18:11:38 Adding tests for image/draw
2018/07/24 18:11:38 Adding tests for image/gif
2018/07/24 18:11:38 Adding tests for cmd/go/internal/modload
2018/07/24 18:11:38 Adding tests for net/http/pprof
2018/07/24 18:11:38 Adding tests for net/url
2018/07/24 18:11:38 Adding tests for crypto/hmac
2018/07/24 18:11:38 Adding tests for cmd/internal/obj/x86
2018/07/24 18:11:38 Adding tests for hash/fnv
2018/07/24 18:11:38 Adding tests for go/internal/srcimporter
2018/07/24 18:11:38 Adding tests for cmd/cover_test
2018/07/24 18:11:38 Adding tests for net/internal/socktest_test
2018/07/24 18:11:38 Adding tests for internal/poll_test
2018/07/24 18:11:38 Adding tests for net/http/cookiejar_test
2018/07/24 18:11:38 Adding tests for cmd/api
2018/07/24 18:11:38 Adding tests for bufio_test
2018/07/24 18:11:38 Adding tests for debug/macho
2018/07/24 18:11:38 Adding tests for log
2018/07/24 18:11:38 Adding tests for os/signal_test
2018/07/24 18:11:38 Adding tests for encoding/base32
2018/07/24 18:11:38 Adding tests for vendor/golang_org/x/crypto/poly1305
2018/07/24 18:11:38 Adding tests for io/ioutil
2018/07/24 18:11:38 Adding tests for cmd/vendor/github.com/google/pprof/internal/binutils
2018/07/24 18:11:38 Adding tests for math/rand_test
2018/07/24 18:11:38 Adding tests for net/smtp
2018/07/24 18:11:38 Adding tests for mime_test
2018/07/24 18:11:38 Adding tests for syscall_test
2018/07/24 18:11:38 Adding tests for crypto/rc4
2018/07/24 18:11:38 Adding tests for crypto/rand_test
  • it should also skip vendor stuff

  • it should ignore tests

  • it links to source code instead of to the package docs for the implemented methods:

screen shot 2018-07-24 at 11 17 14 am

screen shot 2018-07-24 at 11 17 22 am

@bradfitz bradfitz added the help wanted label Aug 1, 2018

@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 Aug 1, 2018

@bradfitz

This comment has been minimized.

Member

bradfitz commented Aug 1, 2018

Retargetting to Go 1.12.

/cc @dmitshur

@agnivade

This comment has been minimized.

Member

agnivade commented Aug 4, 2018

Why does it care about tests?

According to this comment, it is creating a testmain package. I can see it found out some types that implements interfaces coming from test packages.

// Create a "testmain" package for each package with tests.
	for _, pkg := range prog.AllPackages() {
		if testmain := prog.CreateTestMainPackage(pkg); testmain != nil {
			log.Printf("Adding tests for %s", pkg.Pkg.Path())
		}
	}

it should also skip vendor stuff

That is easy.

it links to source code instead of to the package docs for the implemented methods:

One reason might be it also displays private methods. So it cannot link to package docs since private methods are not shown. We should probably fix this to match with ?m=all and show private methods only when this flag is set. Then we can always link to package docs.

Some closing thoughts -

  • The only difference of the "method-set" section is that it shows methods from embedded structs too. So if we do #6127, then this becomes redundant.

  • Additionally, the "implements" section is to be partially tackled by #20131 (That issue solves both implements and implemented by relations, hence partially).

So as I see - the only 2 new sections added by analysis mode are "implements" and "method-set". And we already have some open issues that try to incorporate some of that into main godoc.

We might need to give some thoughts on whether to polish the analysis mode and make it the default, thereby resolving the other open issues. Or do we slowly bring the features of analysis mode by fixing the open issues one by one. (Does that mean "analysis" mode will become redundant ?)

/cc @alandonovan , @griesemer

btw, running analysis mode takes a ridiculous amount of memory if we run it on GOPATH. Just GOROOT is fine. I had to kill the process after it consumed my entire RAM (16GB).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment