Merged
Conversation
- Replace redundant manual doc(cfg(...)) annotations with doc_auto_cfg (merged into doc_cfg on nightly 1.92+, which docs.rs uses) - Add [package.metadata.docs.rs] to aptos-sdk-macros crate - Add targets = ["x86_64-unknown-linux-gnu"] to docs.rs metadata for both crates to speed up builds - Add [lints] workspace config with check-cfg for docsrs to prevent unexpected_cfgs warnings - Both crates inherit workspace lints - Update CI docs job to use nightly with --cfg docsrs for docs.rs parity - Update release workflow verify step to test docs build with nightly and --cfg docsrs, matching what docs.rs will use - Fix aptos-codegen binary to use current_thread tokio runtime (was failing with --all-features due to missing rt-multi-thread) Co-authored-by: Greg Nazario <greg@gnazar.io>
|
Cursor Agent can help with this pull request. Just |
The rust-toolchain.toml specifies channel = "1.90" (stable), which overrides the nightly toolchain installed by dtolnay/rust-toolchain. Since feature(doc_cfg) requires nightly, all cargo doc commands in CI must explicitly use +nightly to bypass the toolchain file override. Co-authored-by: Greg Nazario <greg@gnazar.io>
The docsrs cfg is only passed as a rustdoc flag, not a rustc flag, so the unexpected_cfgs lint never fires during normal compilation. The cfg_attr(docsrs, ...) in lib.rs simply evaluates to false when docsrs is not set, without triggering any warnings. Co-authored-by: Greg Nazario <greg@gnazar.io>
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns crate metadata, doc configuration, and CI/release workflows so aptos-sdk (and aptos-sdk-macros) build documentation successfully in a docs.rs-like environment.
Changes:
- Add/align
package.metadata.docs.rs(and workspace lints) so docs.rs builds use--cfg docsrsand don’t warn oncfg(docsrs). - Remove redundant
#[cfg_attr(docsrs, doc(cfg(...)))]annotations in favor of centralized docs.rs configuration. - Update CI/release workflows to run docs builds with nightly and
--cfg docsrs, and adjustaptos-codegentokio runtime configuration.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/aptos-sdk/src/transaction/authenticator.rs | Removes docs.rs-only doc(cfg(...)) attributes for the keyless-gated API. |
| crates/aptos-sdk/src/lib.rs | Removes docs.rs-only doc(cfg(...)) attribute for the macros re-export. |
| crates/aptos-sdk/src/bin/codegen.rs | Uses #[tokio::main(flavor = "current_thread")] for the codegen binary. |
| crates/aptos-sdk/src/account/ed25519.rs | Removes docs.rs-only doc(cfg(...)) attributes for mnemonic-gated APIs. |
| crates/aptos-sdk/Cargo.toml | Enables workspace lints + adds docs.rs metadata (all-features, docsrs cfg, target). |
| crates/aptos-sdk-macros/Cargo.toml | Enables workspace lints + adds docs.rs metadata (all-features, docsrs cfg, target). |
| Cargo.toml | Adds workspace rust lint config for unexpected_cfgs with check-cfg for docsrs. |
| .github/workflows/release.yml | Adds nightly docs build parity step using --cfg docsrs and cargo +nightly doc. |
| .github/workflows/ci.yml | Switches docs checks to nightly + docsrs cfg; adds macros docs check. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uses: dtolnay/rust-toolchain@master | ||
| with: | ||
| toolchain: "1.90" | ||
| - name: Install Rust nightly (required for doc_auto_cfg and docs.rs parity) |
There was a problem hiding this comment.
The workflow text says nightly is required for doc_auto_cfg, but there’s no #![feature(doc_auto_cfg)] usage anywhere in the repo (aptos-sdk only enables doc_cfg under docsrs). Consider renaming this step to reference doc_cfg (or enable doc_auto_cfg if that’s what you intended to rely on).
Suggested change
| - name: Install Rust nightly (required for doc_auto_cfg and docs.rs parity) | |
| - name: Install Rust nightly (required for doc_cfg/docs.rs parity) |
Comment on lines
+172
to
174
| - name: Install Rust nightly (required for doc_auto_cfg feature) | ||
| uses: dtolnay/rust-toolchain@nightly | ||
|
|
There was a problem hiding this comment.
This step name mentions doc_auto_cfg, but the codebase doesn’t appear to use #![feature(doc_auto_cfg)] (only doc_cfg is enabled for docsrs). Updating the step name to match the actual feature being used would avoid confusion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Configure docs.rs metadata, update doc attributes, and align CI/release workflows to enable successful documentation publishing to docs.rs.
This PR addresses several issues: redundant
doc(cfg)annotations due todoc_auto_cfg(now part ofdoc_cfgon nightly), missing docs.rs metadata foraptos-sdk-macros, lack ofcheck-cfgfordocsrs, and CI/release workflow inconsistencies with docs.rs build environments. It also includes a fix for theaptos-codegenbinary to ensure--all-featuresbuilds pass.