Skip to content

fix(wallet): no debug on Display implementations #1881

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

Conversation

luisschwab
Copy link
Member

@luisschwab luisschwab commented Mar 9, 2025

Description

Closes #1933.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@luisschwab luisschwab marked this pull request as draft March 9, 2025 23:34
@luisschwab luisschwab marked this pull request as ready for review March 9, 2025 23:51
@ValuedMammal ValuedMammal moved this to Needs Review in BDK Wallet Mar 10, 2025
@110CodingP
Copy link

Although this PR is scoped to the wallet, I was wondering if we should also remove the Debug outputs from Display implementations in keychain_txout.rs (line 798), tx_graph.rs (line 228), spk_client.rs (line 24), and file_store/src/lib.rs (line 25)?

@luisschwab
Copy link
Member Author

Although this PR is scoped to the wallet, I was wondering if we should also remove the Debug outputs from Display implementations in keychain_txout.rs (line 798), tx_graph.rs (line 228), spk_client.rs (line 24), and file_store/src/lib.rs (line 25)?

Yes, but those should go on different PRs.

@luisschwab luisschwab force-pushed the fix/no-debug-on-display-impls branch from 328e42f to 946d454 Compare March 13, 2025 18:56
@luisschwab luisschwab marked this pull request as draft March 13, 2025 19:00
@luisschwab luisschwab force-pushed the fix/no-debug-on-display-impls branch from 0536428 to 2f6642f Compare March 21, 2025 19:12
@luisschwab luisschwab marked this pull request as ready for review March 21, 2025 19:17
@luisschwab luisschwab force-pushed the fix/no-debug-on-display-impls branch from 2f6642f to a393147 Compare March 24, 2025 20:07
@luisschwab
Copy link
Member Author

@ValuedMammal I think this is RTM

@luisschwab
Copy link
Member Author

Closing this and moving to bitcoindevkit/bdk_wallet#12

@luisschwab luisschwab closed this Apr 4, 2025
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Wallet Apr 4, 2025
@luisschwab luisschwab removed this from BDK Wallet May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display implementations should not expose Debug output
3 participants