-
Notifications
You must be signed in to change notification settings - Fork 891
Improve project structure and documentation (package docs, use internal
packages)
#1405
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
Comments
internal
package)internal
packages)
Totally fine if this gets picked up in some spare cycles, but it seems like this is just a |
We should move all of |
@Emyrk I think that's an option, but not a necessity. I also think that's too big an architectural change for this issue. I.e. we'd need to re-evaluate what we want to expose and what we don't. How do we want others to see the project/package? The packages originally suggested here should be sufficiently self-contained for a cleanup job. I believe @kylecarbs might also have some thoughts on why we might not want to move |
I'm against using |
I can definitely relate to that @deansheather. IMO there's a balance to be had there. On the one hand, the Go mantra of "a little copying is better than a little dependency" is good to keep in mind here, things are usually made |
A personal anecdote: I was browsing the Tailscale repo today and found their flat structure extremely easy to consume. |
Gonna close for now until we have a more concrete plan. |
Uh oh!
There was an error while loading. Please reload this page.
I was inspired by #1395 and #1398 to take a look at our current go docs.
A few observations:
// Package [name] ...
header to those)internal
, onlycoder/coder
can import themagent.{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 applicableI'm proposing the following changes (in addition to writing more package docs):
buildinfo
=>internal/buildinfo
coder
is built via project build scripts (data injected at build time), anyone importing this package will see empty valuescli
=>internal/cli
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
:coderd/coderdtest
=>internal/coderdtest
coderd/database/databasefake
=>internal/databasefake
provisioner/echo
=>internal/provisioner/echo
pty/ptytest
=>internal/ptytest
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
:The text was updated successfully, but these errors were encountered: