-
Notifications
You must be signed in to change notification settings - Fork 166
docs: Add composefs-rs crates to internals rustdoc #1895
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the documentation build process to include rustdoc for the external composefs-rs git dependencies. The changes modify docs/Dockerfile.mdbook to generate the documentation and docs/src/internals.md to add links to it. This is a good addition as it makes documentation for these key dependencies available locally. The implementation is correct, but I have one suggestion to ensure the generated documentation for the external crates is as comprehensive as for the internal ones.
fc61e35 to
8359c89
Compare
jeckersb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, sanity checked locally. Not sure why the docs job is failing in CI, it works fine for me locally and it's all done in the container build so I don't see why it would be any different in CI...
|
It's because
So cargo-binstall falls back to building from source which for bad reasons also includes building openssl from source which for historical reasons requires Perl stuff we don't ship in the devenv. |
These external git dependencies don't have docs on docs.rs, so include them in the internal documentation alongside our workspace crates. Assisted-by: OpenCode (Opus 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
Add a comment noting that changes to the composefs-rs crate list should also update the documentation files that reference them. Assisted-by: OpenCode (Opus 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
When cargo-binstall fetches pre-built binaries from GitHub, it can hit API rate limits (403 Forbidden) when unauthenticated. This causes it to fall back to building from source, which fails for mdbook-linkcheck because the devenv container lacks openssl-devel and the perl modules needed to build OpenSSL from source. Pass the GitHub Actions token through to the container build as a secret, allowing cargo-binstall to make authenticated requests with higher rate limits. Assisted-by: OpenCode (claude-sonnet-4-20250514) Signed-off-by: Colin Walters <walters@verbum.org>
8359c89 to
2e8d169
Compare
|
I tried adding a commit here to propagate the github token into the podman build, which I think will mean that cargo-binstall provides it when doing fetches which should avoid having us be ratelimited as if we were doing anonymous requests. |
jeckersb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops forgot I have to re-review now 😆
These external git dependencies don't have docs on docs.rs, so include them in the internal documentation alongside our workspace crates.
Assisted-by: OpenCode (Opus 4.5)