Skip to content

deprecate descriptor and change_descriptor fields#394

Open
110CodingP wants to merge 1 commit intobitcoindevkit:masterfrom
110CodingP:deprecate_fields
Open

deprecate descriptor and change_descriptor fields#394
110CodingP wants to merge 1 commit intobitcoindevkit:masterfrom
110CodingP:deprecate_fields

Conversation

@110CodingP
Copy link
Contributor

Description

These fields are planned to be replaced by keyring field when we have the multi-keychain wallet so need to be deprecated following the policy in #391 .

Changelog notice

  • Deprecated network, descriptor and change_descriptor fields of wallet::ChangeSet .

Checklists

All Submissions:

@codecov
Copy link

codecov bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.77%. Comparing base (c422104) to head (bf9a904).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #394      +/-   ##
==========================================
+ Coverage   79.17%   79.77%   +0.60%     
==========================================
  Files          24       24              
  Lines        5311     5266      -45     
  Branches      242      241       -1     
==========================================
- Hits         4205     4201       -4     
+ Misses       1029      988      -41     
  Partials       77       77              
Flag Coverage Δ
rust 79.77% <100.00%> (+0.60%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@luisschwab luisschwab added the api A breaking API change label Mar 4, 2026
@luisschwab luisschwab moved this to Needs Review in BDK Wallet Mar 4, 2026
@luisschwab luisschwab added this to the Wallet 3.0.0 milestone Mar 4, 2026
Since these fields(`descriptor`, `change_descriptor`) will be
replaced by `keyring` when we have the multi-keychain wallet.
@110CodingP 110CodingP changed the title deprecate network, descriptor and change_descriptor fields deprecate descriptor and change_descriptor fields Mar 6, 2026
@Serious-Sam-Dev
Copy link

all test have passed, clippy dont warning, approach ACK, the only thing i see need change is commit name, try use the conventional commits, see more in CONTRIBUTING, change to conventional commit to make more easy to review

@110CodingP
Copy link
Contributor Author

110CodingP commented Mar 7, 2026

all test have passed, clippy dont warning, approach ACK, the only thing i see need change is commit name, try use the conventional commits, see more in CONTRIBUTING, change to conventional commit to make more easy to review

I wasn't sure what type this commit should be classified as 😅 . Should it be docs or refactor?

@Serious-Sam-Dev
Copy link

hmmm... good question, i think docs is better than the refactor, refactor say more change code or remove, maybe (fix), (docs) our (chore) is valid, i believe change the commit name to any of this "tags" will give to PR, more relevance to a view of a maintainer

Copy link
Collaborator

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

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

I guess we can't deprecate a field until we've provided an alternative, i.e. the keyring, which is currently planned for 4.0.

@110CodingP
Copy link
Contributor Author

In that case, should I close this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api A breaking API change

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

4 participants