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

Unify error enums in substrate and ethereum clients with thiserror #1094

Merged
merged 5 commits into from
Oct 22, 2021

Conversation

vmarkushin
Copy link
Contributor

Related to #857

Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

The changes in the current draft look good, but I was hoping we can get rid of all of the String error types and replace them with anyhow or concrete instances.

@vmarkushin
Copy link
Contributor Author

The changes in the current draft look good, but I was hoping we can get rid of all of the String error types and replace them with anyhow or concrete instances.

Yeah, I wasn't certain about all the String errors. Now it's clear, I will deal with the rest.

@tomusdrw
Copy link
Contributor

@vmarkushin any updates on this? I'm okay merging this as-is and leaving the String error types eradication for future PRs.

@vmarkushin vmarkushin marked this pull request as ready for review October 19, 2021 21:59
@vmarkushin vmarkushin requested a review from tomusdrw October 19, 2021 21:59
@vmarkushin
Copy link
Contributor Author

@tomusdrw I worked on the code a bit more. Please check it when you have time.

Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks great, thanks a lot! License pre-amble is missing in new files, but apart from that I'm happy to get that merged.

@@ -0,0 +1,25 @@
use thiserror::Error;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
use thiserror::Error;
// Copyright 2021 Parity Technologies (UK) Ltd.
// This file is part of Parity Bridges Common.
// Parity Bridges Common is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// Parity Bridges Common is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>.
use thiserror::Error;

@@ -0,0 +1,31 @@
use std::net::AddrParseError;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
use std::net::AddrParseError;
// Copyright 2019-2021 Parity Technologies (UK) Ltd.
// This file is part of Parity Bridges Common.
// Parity Bridges Common is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
// Parity Bridges Common is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
// You should have received a copy of the GNU General Public License
// along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>.
use std::net::AddrParseError;

@tomusdrw tomusdrw merged commit b270b6a into paritytech:master Oct 22, 2021
@tomusdrw
Copy link
Contributor

Thanks!!!

@vmarkushin vmarkushin deleted the intro-thiserror branch October 22, 2021 11:24
svyatonik pushed a commit that referenced this pull request Jul 17, 2023
Bumps [syn](https://github.com/dtolnay/syn) from 1.0.88 to 1.0.89.
- [Release notes](https://github.com/dtolnay/syn/releases)
- [Commits](dtolnay/syn@1.0.88...1.0.89)

---
updated-dependencies:
- dependency-name: syn
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
…aritytech#1094)

* Unify error enums in substrate and ethereum clients with `thiserror`

Related to paritytech#857

* Add license pre-amble

* rustfmt

* Fix spelling
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
…aritytech#1094)

* Unify error enums in substrate and ethereum clients with `thiserror`

Related to paritytech#857

* Add license pre-amble

* rustfmt

* Fix spelling
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