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!: migrate from thiserror to snafu #1

Closed
wants to merge 3 commits into from
Closed

Conversation

lgiussan
Copy link
Collaborator

Rationale for this change

See spaceandtimelabs#174 for rationale

What changes are included in this PR?

NOTE: the individual commits may be good to review individually.

  • All error enum variants consisting of tuple structs are transformed into named structs. This is necessary because snafu does not support tuple structs.
  • Every #[derive(Error)] is substituted with the analogous #[derive(Snafu)]. In particular:
    • #[error(...)] attributes are substituted with equivalent #[snafu(display(...))] attributes
    • #[error(transparent)] attributes are substituted with equivalent #[snafu(transparent)] attributes (which also derive the corresponding From implementation)
    • For ConversionError::TimestampConversionError, the #[snafu(context(false), display(...))] attribute is used for deriving a From implementation, and at the same time maintain the custom error message
  • A std feature is introduced for the proof-of-sql crate, which in turns activates the snafu/std feature. The std feature is required for the posql_db example, because Clap relies on the std::error::Error trait.
  • thiserror still appears in the dependency tree (cargo tree -i thiserror), but only as a transitive dependency (via blitzar, and dev-dependencies)

Are these changes tested?

Yes. This PR is a refactoring, and all existing tests pass.

@lgiussan lgiussan changed the title Refactor/snafu refactor!: migrate from thiserror to snafu Sep 25, 2024
@lgiussan lgiussan marked this pull request as draft September 25, 2024 13:17
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.

1 participant