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

Introduce '{Shallow,Deep}CopyBuffer'. #795

Merged
merged 2 commits into from
Jul 12, 2023
Merged

Conversation

1uc
Copy link
Collaborator

@1uc 1uc commented Jul 10, 2023

This commit introduces the internal notion of buffers. We already need to copy non-contiguous data into a contiguous buffer. This commit simply makes this idea more explicit.

The advantage is that each class can focus on its purpose and thereby simplify its code.

@1uc 1uc requested a review from alkino July 10, 2023 13:04
@1uc 1uc marked this pull request as ready for review July 10, 2023 13:04
Copy link
Member

@alkino alkino left a comment

Choose a reason for hiding this comment

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

really nice addition.

This commit introduces the internal notion of buffers. We already
need to copy non-contiguous data into a contiguous buffer. This
commit simply makes this idea more explicit.

The advantage is that each class can focus on its purpose and thereby
simplify its code.
@1uc 1uc force-pushed the 1uc/refactor-copy-buffers branch from f21ca6a to 3c62836 Compare July 12, 2023 09:29
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #795 (bf807d2) into master (b68bfd0) will increase coverage by 0.30%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #795      +/-   ##
==========================================
+ Coverage   84.35%   84.65%   +0.30%     
==========================================
  Files          68       68              
  Lines        4826     4797      -29     
==========================================
- Hits         4071     4061      -10     
+ Misses        755      736      -19     
Impacted Files Coverage Δ
include/highfive/bits/H5Attribute_misc.hpp 79.68% <100.00%> (ø)
include/highfive/bits/H5Converter_misc.hpp 100.00% <100.00%> (ø)
include/highfive/bits/H5Slice_traits_misc.hpp 90.08% <100.00%> (ø)

... and 1 file with indirect coverage changes

alkino
alkino previously approved these changes Jul 12, 2023
include/highfive/bits/H5Converter_misc.hpp Outdated Show resolved Hide resolved
Co-authored-by: Nicolas Cornu <nicolas.cornu@epfl.ch>
@1uc 1uc merged commit 5f71511 into master Jul 12, 2023
30 checks passed
@1uc 1uc deleted the 1uc/refactor-copy-buffers branch July 12, 2023 11:30
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