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

Remove network::Error #1032

Merged

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jun 1, 2022

The network::Error is not used, remove it.

(This description has been changed, the thumbs up emojis were put on the previous PR description.)

sanket1729
sanket1729 previously approved these changes Jun 1, 2022
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

crACK 94680a5.

apoelstra
apoelstra previously approved these changes Jun 1, 2022
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 94680a5

@apoelstra
Copy link
Member

This approach looks good to me, but I should mention two alternatives:

  • Renaming Error::Network to Error::Io
  • Just returning an io::Error directly from all the network funcitons

The `network::Error` is not used, remove it.
@tcharding tcharding dismissed stale reviews from apoelstra and sanket1729 via 99aab44 June 1, 2022 23:50
@tcharding
Copy link
Member Author

Lolz, terrible work by me. The error is not even used, we can just remove it. (Found by exploring your suggestions @apoelstra).

@sanket1729
Copy link
Member

Lolz, terrible work by me. The error is not even used, we can just remove it.

I am also to blame here :)

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

reACK 99aab44

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 99aab44

lol!!

@apoelstra apoelstra merged commit 21f4493 into rust-bitcoin:master Jun 2, 2022
@tcharding tcharding deleted the 06-02-remove-network-error branch June 2, 2022 23:49
ChallengeDev210 pushed a commit to ChallengeDev210/rust-bitcoin that referenced this pull request Aug 1, 2022
99aab44 Remove network::Error (Tobin C. Harding)

Pull request description:

  The `network::Error` is not used, remove it.

  (This description has been changed, the thumbs up emojis were put on the previous PR description.)

ACKs for top commit:
  sanket1729:
    reACK 99aab44
  apoelstra:
    ACK 99aab44

Tree-SHA512: 2342531160966860b7b65f8c5df10e169876ec446e6fd30093d5d81d0b0304cad04e2c2057eb3ca6b23a2fc56453c91ad4ddf426d3796fb301acb7f7d03a66b9
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.

None yet

3 participants