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

Refactor the Warning type #278

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Refactor the Warning type #278

wants to merge 4 commits into from

Conversation

rustaceanrob
Copy link
Owner

@rustaceanrob rustaceanrob commented Jan 23, 2025

Closes #277

Cleaning up the Warning type here to resolve a typo, refactor to clearer names, and add additional context to warnings so the developer might act on them. Also adding some additional context to a HeaderPersistenceError.

Each refactor is broken down commit by commit so it should be okay to review. Only context needed is in the CannotLocateHistory variant, which throws if the user configures a checkpoint that is at a height lower than what is stored in the SQL database on start up. This prevents the user from going backwards if they have already synced before, which is a pretty soft requirement. For the first sync one can start from anywhere. @nyonson

@rustaceanrob
Copy link
Owner Author

rustaceanrob commented Jan 23, 2025

Dude f*** the MSRV. I can't use BTreeMap::first_key_value?? Lol

@rustaceanrob
Copy link
Owner Author

Thought about it for a second. While the last commit would be useful, the load_headers function needs work anyway. Going to remove the last commit from this PR, so the whole context about CannotLocateHistory is no longer relevant for this PR.

@@ -1641,7 +1641,7 @@ dependencies = [

[[package]]
name = "kyoto-cbf"
version = "0.7.0"
version = "0.8.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this some leftovers?

Copy link
Contributor

@nyonson nyonson left a comment

Choose a reason for hiding this comment

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

ACK bb934c3

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.

Warning type could use some work
2 participants