-
Notifications
You must be signed in to change notification settings - Fork 4
Add interval array #60
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
base: main
Are you sure you want to change the base?
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
=======================================
Coverage ? 82.49%
=======================================
Files ? 36
Lines ? 1771
Branches ? 0
=======================================
Hits ? 1461
Misses ? 310
Partials ? 0
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:
|
a5d93e6 to
1b108d4
Compare
| namespace sparrow_ipc | ||
| { | ||
| template <typename T> | ||
| [[nodiscard]] sparrow::interval_array<T> deserialize_non_owning_interval_array( |
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.
It seems this function is exactly the same (except from data type) as the one used in primitive_array, so we should probably refactor and use some generic function.
| const std::string name = field->name() == nullptr ? "" : field->name()->str(); | ||
| const bool nullable = field->nullable(); | ||
| const auto field_type = field->type_type(); | ||
| // TODO rename all the deserialize_non_owning... fcts since this is not correct anymore |
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.
Why remove this? This is actually still relevant as we are using now optionally_owned_buffer (which is not necessarily non owning as the functions names suggest).
|
Should this be linked to the corresponding issue? |
No description provided.