Skip to content
This repository was archived by the owner on Nov 30, 2022. It is now read-only.

Update error implementations for the new MSRV#155

Merged
apoelstra merged 4 commits intorust-bitcoin:masterfrom
tcharding:06-01-error
Oct 21, 2022
Merged

Update error implementations for the new MSRV#155
apoelstra merged 4 commits intorust-bitcoin:masterfrom
tcharding:06-01-error

Conversation

@tcharding
Copy link
Member

Implement source for all error types (there are only two). Done because we have bumped the MSRV. First two patches are preparatory cleanup.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 0ebe201

@tcharding
Copy link
Member Author

tcharding commented Jun 1, 2022

I added an additional patch that adds non_exhaustive to this PR, thought re-review of this would be the easier than doing it as a separate PR.

@tcharding tcharding requested a review from apoelstra June 1, 2022 23:07
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK b5d7c74

The module `impls` holds implementations of traits found in `std` and
`core2` but not in `core`; `io::Read` should be implemented in the
`impls` module.

Move `io::Read` implementation to the `impls` module.
Now that we have bumped the MSRV we should implement `error::Error` as
it is defined for Rust 1.41.1 that means implementing `source` instead
of `cause` and `description`.

We only have two error types, implement `source` for both of them.
As we are doing in `rust-bitcoin`; to enhance forward compatability, and
ease the upgrade path for downstream projects, add `non_exhaustive` to
all error types.
@tcharding
Copy link
Member Author

Rebased to fix merge conflicts, no other changes.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK d366d32

@tcharding
Copy link
Member Author

Is this one going in? If so can we merge and then freeze bitcoin_hashes so we can update and merge rust-bitcoin/rust-bitcoin#1284?

@apoelstra apoelstra merged commit 2a78c25 into rust-bitcoin:master Oct 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants