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

refactor: define custom error values for consensus crate #6

Merged
merged 4 commits into from
Oct 13, 2023
Merged

refactor: define custom error values for consensus crate #6

merged 4 commits into from
Oct 13, 2023

Conversation

moshababo
Copy link
Contributor

@moshababo moshababo commented Oct 11, 2023 •
edited
Loading

Refactoring the consensus crate to define custom error values to be used instead of the on-the-fly, non-formatted anyhow errors.

Closing BFT-308.

brunoffranca
brunoffranca requested changes Oct 11, 2023
View reviewed changes
Copy link
Member

@brunoffranca brunoffranca left a comment

Choose a reason for hiding this comment

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

This part looks good. But we also want the thiserror error messages everywhere in the consensus crate. In fact, in the end you should be able to remove anyhow as a dependency from the consensus crate.

node/actors/consensus/src/leader/error.rs Outdated Show resolved Hide resolved
node/actors/consensus/src/leader/error.rs Outdated Show resolved Hide resolved
node/actors/consensus/src/leader/error.rs Outdated Show resolved Hide resolved
@brunoffranca
Copy link
Member

Another thing, you need to add some information to the PR description. Since the Linear ticket is private, some people will only be able to see the PR description.

@moshababo moshababo changed the title refactor: define custom error values for replica messages handling refactor: define custom predefined error values for consensus crate Oct 13, 2023
@moshababo moshababo changed the title refactor: define custom predefined error values for consensus crate refactor: define custom error values for consensus crate Oct 13, 2023
brunoffranca
brunoffranca approved these changes Oct 13, 2023
View reviewed changes
Copy link
Member

@brunoffranca brunoffranca left a comment

Choose a reason for hiding this comment

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

Perfect. Approved!

@brunoffranca brunoffranca merged commit 1267e79 into main Oct 13, 2023
4 checks passed
@brunoffranca brunoffranca deleted the bft-308 branch October 13, 2023 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brunoffranca brunoffranca brunoffranca approved these changes

Assignees

@moshababo moshababo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@moshababo @brunoffranca