Closed as not planned
Closed as not planned
Description
I was inspired by #1395 and #1398 to take a look at our current go docs.
A few observations:
- Most packages are lacking documentation (we should add the
// Package [name] ...
header to those) - Many test packages are public and can be imported by others as a dependency (we don't want to allow our users to easily break their workflows by exposing APIs that break on a whim)
- By moving these to a path named
internal
, onlycoder/coder
can import them - Renaming these has the added benefit of decreasing API/docs surface and makes it clear they are not public
- By moving these to a path named
- Quite a few types are lacking documentation
- Example:
agent.{Metadata,Options}
, it's a good practice for all types to have documentation, even if their purpose is quite clear. It's also a good chance to document edge cases and behaviors, if applicable
- Example:
I'm proposing the following changes (in addition to writing more package docs):
buildinfo
=>internal/buildinfo
- Motivation: This package only contains useful information when
coder
is built via project build scripts (data injected at build time), anyone importing this package will see empty values
- Motivation: This package only contains useful information when
- (maybe)
cli
=>internal/cli
- Note: Not necessarily the sub-packages
- Motivation: We only export
Root()
which is only useful if someone wants the same functionality as thecmd/coder
cli (I don't think we want anyone using this in their project(s))
cli/clitest
=>internal/clitest
:- Motivation: Only we should be testing that our cli commands behave as expected
coderd/coderdtest
=>internal/coderdtest
- Motivation: Only we should be testing that our APIs behave as expected, we don't want our users to accidentally become dependent on functionality in this package
coderd/database/databasefake
=>internal/databasefake
- Motivation: Only used for testing, not advisable to be used in production
- (maybe)
provisioner/echo
=>internal/provisioner/echo
- Motivation: Are there legit use cases for others to use this, or test-only? If there are no use-cases we may not want to expose this before we know how it will or should be used
pty/ptytest
=>internal/ptytest
- Motivation: Test helper, we don't want others to depend on this package
I mostly took a surface look approach to this, there may be other non-obvious (to me) packages that could be hidden away.
No exploration was done for hiding away package functions and types, but we may want to consider that as well. The fewer things we export, the clearer the project and it's capabilities are.
To view the docs yourself, you can use godoc
:
go install golang.org/x/tools/cmd/godoc@latest
cd /path/to/coder
godoc -http=:6060
# open http://localhost:6060/pkg/github.com/coder/coder/