refactor: replace examples with focused rustdoc examples#2006
refactor: replace examples with focused rustdoc examples#2006kwsantiago wants to merge 1 commit intobitcoindevkit:masterfrom
Conversation
|
Thanks for working on this one. |
bcf49f0 to
7448a99
Compare
|
@oleonardolima thank you. CI "should" be fixed now, at least it is locally. Can we try again? |
0e68ca5 to
9a9f9af
Compare
|
Thanks, there are some unrelated already merged commits, you probably need to do a rebase on top of master. Also, please note that we follow the conventional commits for commit messages, which would also need an update. |
9a9f9af to
dc4e009
Compare
|
@oleonardolima ok should be ready to test CI now. Thanks! |
1f81e2c to
fa85989
Compare
|
You'll need to perform another rebase to incorporate the CI updates/fixes, and remove the unrelated merge commit. |
fa85989 to
6fb8acd
Compare
|
@oleonardolima should be good now, let me know if there is anything else here. |
oleonardolima
left a comment
There was a problem hiding this comment.
You should also remove the example exclusion in build and test CI step.
I'm also wondering if the cfgs are strictly required, but haven't tested anything different yet.
86defb3 to
92ba283
Compare
|
cACK, but haven't done a close review yet. Thanks for hanging in on this one, but looks like it needs another rebase. |
7c98357 to
8d6a3c6
Compare
7116530 to
fe792bf
Compare
|
@kwsantiago you should run |
fe792bf to
9b01fda
Compare
|
@luisschwab got it, thanks! |
9b01fda to
ca0883f
Compare
|
Code Coverage failure is unrelated to this PR: |
|
This will need a rebase also since I merged #2123. |
ca0883f to
f9ec921
Compare
|
@notmandatory I rebased onto master in f9ec921 |
f9ec921 to
fbfe3b5
Compare
ValuedMammal
left a comment
There was a problem hiding this comment.
- Can you change
ignoretono_runinbdk_electrum_client.rs, that way we know the example compiles. - You can remove the
#[allow(deprecated)]fromfetch_latest_blocks. Thanks @kwsantiago
fbfe3b5 to
a98d654
Compare
a98d654 to
012c809
Compare
|
You may need to enable the crate default features for dev-dependencies in |
|
In |
012c809 to
35d6980
Compare
|
@ValuedMammal your comments have been addressed in 35d6980 |
Remove standalone examples in favor of focused, contextual rustdoc examples that are easier to maintain and provide better documentation. This makes the codebase more maintainable while improving the developer experience with examples that are tested alongside the code they document.
35d6980 to
b2a9647
Compare
nymius
left a comment
There was a problem hiding this comment.
I think this is in a very good state, but It's hard to extract from the original issue the requirements to consider this done.
The value of the examples/ was mainly due to the integration of the multiple components bdk implements, and that's hard to replicate by just adding doc examples to the interesting methods.
That's why I think there is no much escape to having to look at bdk-cli for it.
However, there are components missing examples in the docs that will benefit from these changes.
@kwsantiago has already implemented some. The following list is an attempt to extract the provided functionality by the examples/ and identify the missing doc examples:
-
Esploranew:Blocking/AsyncClient::from_builder()
Missing example.
-
Esplorasync:EsploraAsync/EsploraExt::sync()
Missing example.
-
Esplorafull scan:EsploraAsync/EsploraExt::full_scan() -
Electrumnew:BdkElectrumClient::new() -
Electrumsync:BdkElectrumClient::sync() -
Electrumfull scan:BdkElectrumClient::full_scan() -
Electrumparameter:batch_size. -
bitcoind_rpcsync/live:Emitter::next_block()andEmitter::mempool
Missing example.
-
TxOutlisting:TxGraph::try_canonical_view+(filter_outpointsfilter_outpointsalready contains examples)
Missing example.
-
BalanceTxGraph::balance() -
Indexsetup:KeychainTxOutIndex::insert_descriptor() -
Addresslisting:KeychainTxOutIndex::revealed_keychain_spks() -
Addressreveal next:KeychainTxOutIndex::reveal_next_spk() -
Addressreveal unused:KeychainTxOutIndex::next_unused_spk()
Once we have these in place, the added examples together with the bdk-cli project are going to be a better guide than the old examples/.
I left outside of the above list the functionality that cannot be replicated without recreating end to end examples like before.
- Wallet setup: generate keys, initialize indexes and local chain, set network and database. Trying to replicate this in some way will fall back again to the old
examples/way. It's better to point users tobdk-cliinstead. PSBToperations: new, sign and extract: PSBT creation is mainly managed throughTxBuilderinbdk_wallet,signis on its way to be removed and extraction is also done bybdk_wallet. For more fine grained control is better to look atbdk_tx.- Coin selection and transaction building: Same than above, most of this is implemented with
TxBuilderinbdk_wallet, and for the new interface is better to work onbdk-tx.
| /// use bdk_chain::miniscript::{Descriptor, DescriptorPublicKey}; | ||
| /// # use std::str::FromStr; | ||
| /// | ||
| /// let mut index = KeychainTxOutIndex::<&str>::new(10, true); |
There was a problem hiding this comment.
I would like to hear other's opinions, but wouldn't it be better to replace all these uses of raw integers as parameters by the name of the parameter itself, as we are talking of documentation?
There was a problem hiding this comment.
I agree, having the variable named after the parameter would be more readable than an unexplained value.
@nymius Thanks for the detailed comment and review! If I may add to it, I would suggest to break this into different PRs if possible, e.g one for bdk_esplora; one for bdk_electrum, and so on.
Yes, it's fine to left this on the side for now, they're |
Closes #1973
Changes
/examplesfolder containing CLI-heavy examplesIndexedTxGraph::new()- graph initializationBdkElectrumClient::new()- client creationBdkElectrumClient::sync()- blockchain syncBdkElectrumClient::full_scan()- wallet restorationTxGraph::insert_tx()- transaction insertionTxGraph::balance()- balance calculationTxGraph::filter_chain_unspents()- UTXO retrievalKeychainTxOutIndex::reveal_next_spk()- address generationKeychainTxOutIndex::insert_descriptor()- descriptor setupIndexedTxGraph::apply_block_relevant()- block processingEsploraExt::full_scan()- Esplora wallet scanningCargo.tomlworkspace membersRationale
The previous examples contained 300+ lines of CLI boilerplate that obscured the core BDK functionality. The new rustdoc examples are 10-15 lines each and focus purely on API usage, making them much easier for developers to understand and follow.
The maintained
bdk-clitool serves as the comprehensive CLI example.