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.

Sign up for GitHub

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

Jump to bottom

Implement std::error::Error::source #72

Merged
merged 2 commits into from
Aug 22, 2022
Merged

Implement std::error::Error::source #72

merged 2 commits into from
Aug 22, 2022

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jul 28, 2022 •
edited
Loading

Now that we have MSRV of 1.41.1 we can implement source to improve the ergonomics of our errors.

Patch 1 is preparatory cleanup.

@tcharding tcharding force-pushed the 07-28-errors branch 3 times, most recently from 4125c4a to 5e0bf8a Compare July 28, 2022 06:35
Kixunil
Kixunil approved these changes Jul 28, 2022
View reviewed changes
Copy link

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK 5e0bf8a

Kixunil
Kixunil approved these changes Jul 29, 2022
View reviewed changes
Copy link

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK b7416fc

clarkmoody
clarkmoody approved these changes Aug 1, 2022
View reviewed changes
Copy link
Member

@clarkmoody clarkmoody left a comment

Choose a reason for hiding this comment

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

ACK b7416fc

@tcharding
Copy link
Member Author

Changes in force push:

  • Update source method to include new error variant.

Kixunil
Kixunil approved these changes Aug 10, 2022
View reviewed changes
Copy link

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK 6b8e8f2

@tcharding
Copy link
Member Author

Force push to trigger CI, no changes.

@tcharding
Copy link
Member Author

CI fail is a network error

E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/g/gst-plugins-base1.0/gstreamer1.0-x_1.16.3-0ubuntu1_amd64.deb  404  Not Found [IP: 52.252.75.106 80]
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?

@Kixunil
Copy link

Kixunil commented Aug 11, 2022

@tcharding FYI maintainers can re-run failed jobs without pushing. Just open the details of it and there's a button in top right corner.

@tcharding
Copy link
Member Author

I'm not a maintainer on this repo. I'm confident to take on that added responsibility though at @clarkmoody and/or @apoelstra's blessing (preferably both). I will not take any offense if the offer is not taken up.

@tcharding tcharding mentioned this pull request Aug 11, 2022
Merged
apoelstra
apoelstra approved these changes Aug 11, 2022
View reviewed 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 b7416fc

@apoelstra
Copy link
Member

I kicked CI. Let's get some API fixes in and then we'll revisit maintainership. It's fine with me in principle.

@tcharding
Copy link
Member Author

tcharding commented Aug 11, 2022 •
edited
Loading

Something is wrong with CI, there is a network fail that has been happening for a few days now.

Oh its not network, the VM is failing to find specific packages

E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/g/gst-plugins-base1.0/libgstreamer-plugins-base1.0-0_1.16.3-0ubuntu1_amd64.deb  404  Not Found [IP: 40.119.46.219 80]

@tcharding
Copy link
Member Author

Attempted ci fix in #76

@apoelstra
Copy link
Member

Merged #76. Wanna rebase this?

@tcharding
Add full stops to error rustdocs
6052c60 
In preparation for improving the error code make all the rustdocs
complete sentences by adding a full stop.
@tcharding
Copy link
Member Author

Rebased, on other changes.

@apoelstra
Copy link
Member

lol @tcharding could you fix the cargo fmt failure?

@tcharding
Copy link
Member Author

Face palm :)

@tcharding
Implement std::error::Error::source
5b520d0 
Now that we have MSRV of 1.41.1 we can implement `source` to improve the
ergonomics of our errors.
apoelstra
apoelstra approved these changes Aug 22, 2022
View reviewed 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 5b520d0

@apoelstra apoelstra merged commit ae9f18a into rust-bitcoin:master Aug 22, 2022
@tcharding tcharding deleted the 07-28-errors branch August 25, 2022 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kixunil Kixunil Kixunil approved these changes

@apoelstra apoelstra apoelstra approved these changes

@clarkmoody clarkmoody clarkmoody approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tcharding @Kixunil @apoelstra @clarkmoody