-
Notifications
You must be signed in to change notification settings - Fork 285
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
Add Metaskeleton::cloneMetaSkeleton() #1201
Conversation
Next: * Simplify clone functions utilizing Linkage only requires one skeleton clone
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.
LGTM 👍
I just left one very minor nitpick about a comment that I think might be out of date.
dart/dynamics/MetaSkeleton.hpp
Outdated
const std::string& cloneName) const = 0; | ||
// TODO: In DART7, rename this to clone() and change the current | ||
// Skeleton::clone() to override it. Then consider adding | ||
// Skeleton::cloneSkeleton(). |
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.
Super nitpick: We don't need to consider adding Skeleont::cloneSkeleton()
, because it's already been added.
Codecov Report
@@ Coverage Diff @@
## release-6.7 #1201 +/- ##
===============================================
+ Coverage 56.59% 56.65% +0.06%
===============================================
Files 346 346
Lines 25658 25812 +154
===============================================
+ Hits 14520 14625 +105
- Misses 11138 11187 +49
|
This PR introduces clone functions to MetaSkeleton classes. It basically clones necessary
Skeleton
s are held by a MetaSkeleton as needed.The clone function name of MetaSkeleton is
cloneMetaSkeleton()
to avoid the name conflict withSkeleton::clone()
.Before creating a pull request
clang-format
Before merging a pull request
CHANGELOG.md