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

add doc_auto_cfg to relevant sdk crates #3121

Merged
merged 4 commits into from
Oct 12, 2024

Conversation

kevinheavey
Copy link

@kevinheavey kevinheavey commented Oct 9, 2024

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 and solana_program because those crates enable almost all their features by default.

Summary of Changes

For each crate in sdk/ where this is an issue, add

all-features = true
rustdoc-args = ["--cfg=docsrs"]

to [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/

yihau
yihau previously approved these changes Oct 10, 2024
Copy link
Member

@yihau yihau left a 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!

@yihau
Copy link
Member

yihau commented Oct 10, 2024

if don't mind, we can wait @joncinque or @febo to have a chance to take a look

Copy link

@joncinque joncinque left a 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))]

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?

@@ -1,3 +1,4 @@
#![cfg_attr(docsrs, feature(doc_auto_cfg))]

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))]

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?

Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@joncinque joncinque added the automerge automerge Merge this Pull Request automatically once CI passes label Oct 11, 2024
Copy link

mergify bot commented Oct 11, 2024

automerge label removed due to a CI failure

@mergify mergify bot removed the automerge automerge Merge this Pull Request automatically once CI passes label Oct 11, 2024
@joncinque joncinque added the automerge automerge Merge this Pull Request automatically once CI passes label Oct 11, 2024
Copy link

mergify bot commented Oct 11, 2024

automerge label removed due to a CI failure

@mergify mergify bot removed the automerge automerge Merge this Pull Request automatically once CI passes label Oct 11, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
@kevinheavey kevinheavey added the automerge automerge Merge this Pull Request automatically once CI passes label Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 12, 2024
@kevinheavey
Copy link
Author

@joncinque could you merge please? Looks like automerge is still disabled

kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 19, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
yihau pushed a commit that referenced this pull request Oct 22, 2024
* 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
febo pushed a commit that referenced this pull request Oct 22, 2024
* 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
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 22, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 24, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 26, 2024
mergify bot pushed a commit that referenced this pull request Oct 28, 2024
* extract packet crate

* typo

* sort deps

* update digest

* add doc_auto_cfg like in #3121

* fix frozen-abi
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 28, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 28, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 28, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 29, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Oct 30, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 1, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 1, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 2, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 5, 2024
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 5, 2024
mergify bot pushed a commit that referenced this pull request Nov 6, 2024
* 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>
kevinheavey added a commit to kevinheavey/agave that referenced this pull request Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge automerge Merge this Pull Request automatically once CI passes need:merge-assist
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants