Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

added ability to create hard links #762

Merged
1uc merged 2 commits into from
May 26, 2023
Merged

added ability to create hard links #762

1uc merged 2 commits into from
May 26, 2023

Conversation

@Quark-X10
Copy link
Contributor

@Quark-X10 Quark-X10 commented May 26, 2023

Description

  • added a method in NodeTraits to create hard links

How to test this?

cmake ..
make -j8
make test

Test System

  • OS: Ubuntu 22.04.2 LTS (WSL)
  • Compiler: gcc 11.3.0
  • Dependency versions: hdf5 1.10.7

@alkino
Copy link
Member

alkino commented May 26, 2023

Should fix #357 ?

@Quark-X10
Copy link
Contributor Author

Quark-X10 commented May 26, 2023

I think it should fix #357
I let a typo slip into my PR, sorry for that. Actually this is my first PR ever.

@codecov
Copy link

codecov bot commented May 26, 2023 •
edited
Loading

Codecov Report

Merging #762 (b4c5d81) into master (4573fd3) will increase coverage by 0.05%.
The diff coverage is 95.45%.

@@            Coverage Diff             @@
##           master     #762      +/-   ##
==========================================
+ Coverage   83.62%   83.68%   +0.05%     
==========================================
  Files          66       66              
  Lines        4496     4518      +22     
==========================================
+ Hits         3760     3781      +21     
- Misses        736      737       +1
Impacted Files Coverage Δ
include/highfive/bits/H5Node_traits.hpp 100.00% <ø> (ø)
include/highfive/bits/H5Node_traits_misc.hpp 86.99% <85.71%> (-0.08%) ⬇️
tests/unit/tests_high_five_base.cpp 99.66% <100.00%> (+<0.01%) ⬆️

@1uc
Copy link
Collaborator

1uc commented May 26, 2023

Actually this is my first PR ever.

In that case: Well done! It includes a sensible test which is very good. The only things I'd like to change is the description, since I never know which way links go. We'll fix the formatting issue. Clang-format is a bit particular about using exactly the correct version or otherwise the output differs.

@Quark-X10
Copy link
Contributor Author

Quark-X10 commented May 26, 2023

Thanks !

We'll fix the formatting issue.

I committed the formatting change before seeing your comment sorry.

The only things I'd like to change is the description, since I never know which way links go.

I agree with you, I'm always confused about links direction, if "target object" and "link name" are not clear enough, feel free to use a better wording !

alkino
alkino reviewed May 26, 2023
View reviewed changes
@1uc
Copy link
Collaborator

1uc commented May 26, 2023

Thank you, this looks good. I'll merge it into a branch hard-links, add a few additional tests. Mostly to check if I understand it correctly.

@1uc 1uc changed the base branch from master to hard-links May 26, 2023 13:27
@1uc 1uc merged commit a07e759 into BlueBrain:hard-links May 26, 2023
@1uc 1uc mentioned this pull request May 26, 2023
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@alkino alkino alkino left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Quark-X10 @alkino @1uc