Clean up docs warnings and add CI gate#2189
Conversation
containers-image-proxy 0.10.0 bumps its oci-spec dependency from 0.8 to 0.9. This required a cascade of dependency updates to avoid having two incompatible versions of oci-spec in the dependency graph: - ocidir 0.7.0 → 0.7.2 (adds oci-spec 0.9 support) - composefs-rs rev 54d248f7a7 → 4dd43a107e (0.3.0 → 0.4.0, which also uses containers-image-proxy 0.10 and oci-spec 0.9). The package was also renamed from cfsctl to composefs-ctl. Adapted to breaking API changes in containers-image-proxy (ImageProxyConfig is now #[non_exhaustive]). Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: John Eckersberg <jeckersb@redhat.com>
github-actions
Bot
added
area/system-reinstall-bootc
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates several dependencies, primarily renaming cfsctl to composefs-ctl and updating related composefs crates and containers-image-proxy. It also includes widespread documentation improvements, such as wrapping URLs in angle brackets and fixing formatting in doc comments. Feedback points out an inconsistency in the dependency rename within the root Cargo.toml and suggests using specific prefixes or qualifiers to properly fix intra-doc links rather than removing them.
| # cfsctl = { path = "/path/to/composefs-rs/crates/cfsctl" } | ||
| # The Justfile will auto-detect these and bind-mount them into container builds. | ||
| cfsctl = { git = "https://github.com/composefs/composefs-rs", rev = "54d248f7a7", package = "cfsctl" } | ||
| cfsctl = { git = "https://github.com/composefs/composefs-rs", rev = "4dd43a107e", package = "composefs-ctl" } |
There was a problem hiding this comment.
The dependency rename from cfsctl to composefs-ctl appears inconsistent across the workspace. While Cargo.lock shows that dependencies have been renamed to composefs-ctl in the dependency lists of various crates, this root Cargo.toml still defines the workspace dependency key as cfsctl. This will likely cause build failures in crates that have been updated to use composefs-ctl = { workspace = true }, or result in an inconsistent lock file. You should rename the key here to match the intended package name and update all member crates (such as ostree-ext) to use the new name.
| cfsctl = { git = "https://github.com/composefs/composefs-rs", rev = "4dd43a107e", package = "composefs-ctl" } | |
| composefs-ctl = { git = "https://github.com/composefs/composefs-rs", rev = "4dd43a107e" } |
Escape bare URLs with angle brackets, wrap HTML-like tokens in backticks or parentheses, escape brackets in doc comments, and remove broken intra-doc links to private or out-of-scope items. Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Broaden the existing 'cargo doc --lib' in the Makefile validate target to '--workspace --no-deps --document-private-items' and set RUSTDOCFLAGS='-D warnings' so that any rustdoc warnings (broken links, unclosed HTML tags, bare URLs, etc.) will fail CI. Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: John Eckersberg <jeckersb@redhat.com>
| (cd crates/lib && cargo check --no-default-features) | ||
| cargo clippy -- $(CLIPPY_CONFIG) | ||
| env RUSTDOCFLAGS='-D warnings' cargo doc --lib | ||
| env RUSTDOCFLAGS='-D warnings' cargo doc --workspace --no-deps --document-private-items |
There was a problem hiding this comment.
Ah right this basically needs to be kept in sync with the docs build right? Probably worth cross-referencing or deduplicating
|
Turned off auto-merge since this includes part of #2188 so I want to merge that one first and then rebase this otherwise I think it's going to get a bit messy |
2 participants
(Loosely depends on #2188 as that triggered this)
We've accumulated a lot of noisy warnings, fix those and gate on it so we don't accumulate more in the future.