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

Minimal docs. #8

Closed
eddyb opened this issue Nov 15, 2022 · 2 comments · Fixed by #16
Closed

Minimal docs. #8

eddyb opened this issue Nov 15, 2022 · 2 comments · Fixed by #16
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@eddyb
Copy link
Contributor

eddyb commented Nov 15, 2022

Before we release 0.1.0 on crates.io, we should make sure that everything is at least minimally documented.
That is, going to https://docs.rs/spirt/0.1.0 shouldn't look like a ghost town once we publish.

https://embarkstudios.github.io/spirt/spirt/ hosts CI-generated cargo doc output, so you can see what it would look like if we were to publish right away.


Also, due to the use of macros for interned/entity types, some doc comments would go in unfortunate places, e.g. ControlRegion is only documented on ControlRegionDef, as the former is macro-declared in the context module.
It would be great if ControlRegion linked to ControlRegionDef or, even better, somehow held the docs itself.

It might be possible to attach docs to a reexport (instead of the original definition), in which case we could have e.g. pub use crate::context::ControlRegion; just above pub struct ControlRegionDef, and document the former.

@eddyb eddyb added the documentation Improvements or additions to documentation label Nov 15, 2022
@eddyb eddyb added this to the 0.1.0 milestone Nov 15, 2022
@eddyb
Copy link
Contributor Author

eddyb commented Nov 18, 2022

It might be possible to attach docs to a reexport (instead of the original definition)

Turns out it is possible - though only recently (thanks @jyn514 for the tip!):

@eddyb eddyb changed the title Minimal docs enforcement (e.g. with deny(missing_docs)). Minimal docs. Dec 16, 2022
@eddyb
Copy link
Contributor Author

eddyb commented Dec 16, 2022

While working on #16, learned that deny(missing_docs) is overkill (it mandates docs everywhere, including on fields, methods, associated types, etc.) - and IMO that's not what we should be focusing on for 0.1.0.

@eddyb eddyb closed this as completed in #16 Dec 16, 2022
eddyb added a commit that referenced this issue Dec 16, 2022
Closes #8 - the additions are pretty sparse, but the result should be
far easier to navigate.

## Preview (top-level)

![image](https://user-images.githubusercontent.com/77424/208054091-4dcd5194-3c74-499e-b8b7-b8dfbd5dc5f2.png)

## Preview (`ControlRegion` - preexisting docs, but now extra linkified)

![image](https://user-images.githubusercontent.com/77424/208054357-a0715114-c895-4d12-b90a-340204406082.png)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant