-
Notifications
You must be signed in to change notification settings - Fork 8
feat: Add array clone and discard ops #2100
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/arrays #2100 +/- ##
===============================================
+ Coverage 82.96% 82.98% +0.01%
===============================================
Files 223 225 +2
Lines 41839 42022 +183
Branches 37865 38048 +183
===============================================
+ Hits 34713 34870 +157
- Misses 5321 5343 +22
- Partials 1805 1809 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Looks good other than construction confusions
|
||
impl<AK: ArrayKind> GenericArrayClone<AK> { | ||
/// Creates a new array clone op. | ||
pub fn new(elem_ty: Type, size: u64) -> Option<Self> { |
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.
should this return Result
rather than option?
at the very least docstring should explain the return type
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.
typo + formatting suggestion
impl<AK: ArrayKind> GenericArrayClone<AK> { | ||
/// Creates a new array clone op. | ||
/// | ||
/// Returns an error if the proveded element type is not copyable. |
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.
/// Returns an error if the proveded element type is not copyable. | |
/// # Errors | |
/// If the provided element type is not copyable. |
See #2097 for context