Skip to content

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

Merged
apoelstra merged 2 commits intorust-bitcoin:masterfrom
tcharding:07-28-errors
Aug 22, 2022
Merged

Implement std::error::Error::source#72
apoelstra merged 2 commits intorust-bitcoin:masterfrom
tcharding:07-28-errors

Conversation

@tcharding
Copy link
Copy Markdown
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
Copy Markdown

@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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
Member Author

tcharding commented Aug 10, 2022

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
Copy Markdown

@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
Copy Markdown
Member Author

tcharding commented Aug 10, 2022

Force push to trigger CI, no changes.

@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Aug 10, 2022

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
Copy Markdown

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
Copy Markdown
Member Author

tcharding commented Aug 11, 2022

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
Copy Markdown
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
Copy Markdown
Member

apoelstra commented Aug 11, 2022

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
Copy Markdown
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
Copy Markdown
Member Author

tcharding commented Aug 11, 2022

Attempted ci fix in #76

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Aug 12, 2022

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
Copy Markdown
Member Author

tcharding commented Aug 14, 2022

Rebased, on other changes.

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Aug 22, 2022

lol @tcharding could you fix the cargo fmt failure?

@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Aug 22, 2022

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
Copy Markdown
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

@apoelstra apoelstra apoelstra approved these changes

@clarkmoody clarkmoody clarkmoody approved these changes

+1 more reviewer

@Kixunil Kixunil Kixunil approved these changes

Reviewers whose approvals may not affect merge requirements

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