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

Cleanup internals #101

Merged
merged 5 commits into from
Aug 16, 2023
Merged

Conversation

ten3roberts
Copy link
Contributor

As part of getting to know the codebase I've started to do some spring cleaning

@@ -22,7 +22,7 @@ thiserror = "1.0"
trybuild = { version = "1.0.19", features = ["diff"] }
backtrace = "0.3.46"
anyhow = "1.0.28"
syn = { version = "1.0", features = ["full"] }
syn = { version = "2.0", features = ["full"] }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what caused the failure of the src/macros.rs

The two versions of syn were conflicting with thiserror

@ten3roberts
Copy link
Contributor Author

Bringing this up to speed with recent changes in rustc using, such as renamed lints.

Feel free to comment

@ten3roberts ten3roberts requested a review from yaahc August 15, 2023 22:29
src/macros.rs Outdated
@@ -40,7 +40,6 @@
///
/// # fn main() -> Result<()> {
/// # let depth = 0;
/// # let err: &'static dyn std::error::Error = &ScienceError::RecursionLimitExceeded;
Copy link
Collaborator

@yaahc yaahc Aug 15, 2023

Choose a reason for hiding this comment

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

I was looking at this for a minute trying to figure out why it was there... honestly still not really sure. I'm guessing it has something to do with asserting that ScienceError type implements error, but the bail! macro should do the same thing, sooooo.... Seems fine

edit: seeing you already restored, kinda curious if you figured out why it was necessary.

@yaahc yaahc merged commit 2d984da into eyre-rs:master Aug 16, 2023
23 of 28 checks passed
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