-
Notifications
You must be signed in to change notification settings - Fork 49
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
don't use std abs_diff, put it in test_utils instead, run tests with msrv in action #596
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice thanks LGTM! (blocking 0.6 release on this PR)
@@ -106,3 +106,13 @@ pub fn into_record_batch(engine_data: Box<dyn EngineData>) -> RecordBatch { | |||
.unwrap() | |||
.into() | |||
} | |||
|
|||
/// We implement abs_diff here so we don't have to bump our msrv. | |||
/// TODO: Remove and use std version when msrv >= 1.81.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a tracking issue for this so we don't forget?
(really it would be neat if rust had a way to annotate these sorts of workarounds, so they start failing once MSRV bumps)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call :)
What changes are proposed in this pull request?
How was this change tested?