search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.32k stars 17.58k forks source link

x/tools/internal/event: improve package documentation #36238

Open robpike opened 4 years ago

robpike commented 4 years ago

I know it's an internal package, but the package doc for internal/telemetry is poorly conceived:

Package telemetry provides an opinionated set of packages that cover the
main concepts of telemetry in an implementation agnostic way. As a library
author you should look at...

It doesn't say what the package does. It claims that the package is "opinionated' (a passive-agressive way to say, we don't care what you think about the design) but gives no hint at what that opinion leads to. Avoid editorializing, just say what the package does. And it wouldn't hurt to say what "telemetry" is and what it's measuring, and for whom.

dmitshur commented 4 years ago

Thanks for making this improvement suggestion.

For reference, before the telemetry packages were moved out of internal/lsp in CL 190403, the package comment was:

// Package telemetry provides the hooks and adapters to allow use of telemetry
// throughout gopls.
package telemetry

I suspect it's challenging to describe what package telemetry does because "telemetry" is a very broad term covering a large problem space. Perhaps it can be said that the package provides common base types that are used by other golang.org/x/tools/internal/telemetry/... packages.

/cc @ianthehat

robpike commented 4 years ago

The original comment was more helpful. I'm curious why it was changed.

goku321 commented 4 years ago

Can I help in fixing this?

stamblerre commented 4 years ago

The telemetry package has now been moved to internal/event and has a different doc comment, which no longer uses the word "opinionated".

// Package event provides a set of packages that cover the main
// concepts of telemetry in an implementation agnostic way.

As the package is still changing, I think it's reasonable to close this issue.

Unless, @ianthehat, do you want to leave this open to track improving the package documentation more generally? I imagine there will be a lot of docs to write before the package is moved out of internal.

robpike commented 8 months ago

The package doc is still unhelpful. Who is expected to use it, does it honor any standards, the word "agnostic" is somewhat inappropriate, no statement of whose implementation is it agnostic to, and so.

findleyr commented 7 months ago

Coincidentally, I think @ianthehat is in the process of removing this package, replacing it with slog. So this package may go away soon anyway.