-
Notifications
You must be signed in to change notification settings - Fork 200
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
add doc_auto_cfg to relevant sdk crates #3121
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.
looks quite good to me! thank you!
if don't mind, we can wait @joncinque or @febo to have a chance to take a look |
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.
Thanks for noticing and fixing this! Just a few little things to clean up
@@ -1,4 +1,5 @@ | |||
#![no_std] | |||
#![cfg_attr(docsrs, feature(doc_auto_cfg))] |
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.
Can you also add the change to sdk/hash/Cargo.toml
for all-features
and rustdoc-args
?
sdk/instruction/src/error.rs
Outdated
@@ -1,3 +1,4 @@ | |||
#![cfg_attr(docsrs, feature(doc_auto_cfg))] |
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.
Is this required here since it's already enabled in lib.rs?
@@ -1,5 +1,6 @@ | |||
//! 64-byte signature type. | |||
#![no_std] | |||
#![cfg_attr(docsrs, feature(doc_auto_cfg))] |
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.
Can you add the appropriate flags to sdk/signature/Cargo.toml
?
36c6426
to
a06f515
Compare
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.
Looks great, thanks!
automerge label removed due to a CI failure |
a06f515
to
8ac2ec3
Compare
automerge label removed due to a CI failure |
8ac2ec3
to
e2a47f5
Compare
@joncinque could you merge please? Looks like automerge is still disabled |
* extract account-decoder-client-types and convert UiAccount::encode to a free func encode_ui_account * rip out more types and replace some usage in rpc-client and rpc-client-api * rip out more types * remove remaining account-decoder usage from rpc-client * remove account-decoder from rpc-client-api and transaction-status-client-types * make zstd optional * fix import * ignore some lint * update some conversions * missing import * missing import * unused imports * fmt * don't feature-gate any variants of UiAccountEncoding * fmt * update newer code that used UiAccount::encode * remove sdk dep from account-decoder-client-types * add doc_auto_cfg like in #3121
* extract transaction-error crate * update TransactionError usage in sdk * fix frozen-abi support * make serde optional in solana-transaction-error * update lock file * fmt * add #[cfg(not(target_os = "solana"))] where applicable * fmt * put solana-transaction-error behind feature "full" * use workspace lints * update digest * make rustc_version optional * avoid frozen-abi build script * add back workspace lints * remove thiserror * remove unused build deps * use solana-instruction directly * move AddressLoaderError and SanitizeMessageError to transaction-error, and fix its solana-instruction dep features * lint * move TransportError to transaction-error crate * use doc_auto_cfg like in #3121 * missing comma * add deprecations to re-exports
* extract packet crate * typo * sort deps * update digest * add doc_auto_cfg like in #3121 * fix frozen-abi
* extract signer crate * fmt * fix bs58 std usage * extract seed-derivable crate * extract seed-phrase crate * fix copy-pasted doc * extract presigner crate * make keypair module optional in solana-signer * remove serde_json dep and fix test features * remove thiserror from solana-signer * lint * more lint * fix dev dep feature * sort deps * remove itertools from solana-signer * move wasm impl to signer crate * add doc_auto_cfg like in #3121 * post-rebase fix * another post-rebase fix * fmt * make keypair functionality of solana-seed-phrase optional * fix feature-gated imports * Extract solana-keypair, move other things around * Cleanup things that I missed * Remove space * Add deprecated notice * Add deprecation notices * Refactor SeedDerivable trait implementation --------- Co-authored-by: Jon C <me@jonc.dev>
Problem
The new SDK crates put a bunch of code behind feature gates, which means those items won't be visible on docs.rs. This wasn't a big issue with
solana_sdk
andsolana_program
because those crates enable almost all their features by default.Summary of Changes
For each crate in
sdk/
where this is an issue, addto
[package.metadata.docs.rs]
and add
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
to each module that contains feature-gated public items.This makes docs.rs show these items with a note that they require certain features (or targets etc). This solution is fairly common: tokio is a prominent crate that does this.
To emulate what docs.rs generates, you can cd to the individual crate directory and run
RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc --all-features --no-deps
I published a toy example crate to demonstrate that this works on docs.rs: https://docs.rs/doc-feature-examples/latest/doc_feature_examples/