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

Change From<Network> for Currency to TryFrom #2789

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Kixunil
Copy link
Contributor

@Kixunil Kixunil commented Dec 12, 2023

Since Network is non_exhaustive the conversion may fail and thus TryFrom is the appropriate trait to implement.

Notably this doesn't break anything within the crates but it's still API-breaking change (for downstream users).

Since `Network` is `non_exhaustive` the conversion may fail and thus
`TryFrom` is the appropriate trait to implement.
@TheBlueMatt
Copy link
Collaborator

I'd ~infinitely rather drop Currency entirely instead. Lets finish the discussion at rust-bitcoin/rust-bitcoin#2225 before you start opening passive aggressive PRs, thanks.

@Kixunil
Copy link
Contributor Author

Kixunil commented Dec 12, 2023

Sorry if it came off as passive aggressive, I didn't intend that. My intention was:

  • To check how bad of an issue it is
  • To help with any code changes needed
  • Once I found how small change it is to demonstrate it's small in a way that's immediately usable

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.

2 participants