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

feat: Change to use core::error::Error #706

Closed
wants to merge 2 commits into from

Conversation

sorairolake
Copy link

Change to use core::error::Error instead of std::error::Error. The MSRV is bumped up to 1.81.0 by this change.

Copy link

codecov bot commented Sep 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.7%. Comparing base (4a74924) to head (df0491c).
Report is 44 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #706     +/-   ##
=======================================
- Coverage   97.8%   97.7%   -0.1%     
=======================================
  Files         81      83      +2     
  Lines       9378    8974    -404     
=======================================
- Hits        9169    8767    -402     
+ Misses       209     207      -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jhpratt
Copy link
Member

jhpratt commented Sep 23, 2024

This violates the MSRV policy in the README. Additionally, I am very deliberate about bumping MSRV and track a number of features to ensure that I'm not increasing it for only a single thing.

@jhpratt jhpratt closed this Sep 23, 2024
@jhpratt jhpratt added C-invalid Category: no issue exists or the issue cannot be reproduced A-core Area: anything not otherwise covered labels Sep 23, 2024
@sorairolake
Copy link
Author

sorairolake commented Sep 23, 2024

I think this will no longer be a violation of the MSRV policy once Rust 1.83 is released. So I think this can be kept as draft pull request.

@jhpratt
Copy link
Member

jhpratt commented Sep 23, 2024

I do not intend to bump the MSRV the moment 1.83 is out. I will batch a number of changes together and do it all at once, not piecemeal. That's not to mention the countless changes made in this PR for unclear reasons.

@sorairolake
Copy link
Author

That's not to mention the countless changes made in this PR for unclear reasons.

@jhpratt This is because the CI will not pass unless apply cargo fmt to unchanged files too. This is unrelated to this change, so I separated it out as df0491c.

@time-rs time-rs locked as resolved and limited conversation to collaborators Sep 23, 2024
@sorairolake sorairolake deleted the use-core-error branch September 23, 2024 06:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-core Area: anything not otherwise covered C-invalid Category: no issue exists or the issue cannot be reproduced
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants