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

Update API to support fcl 0.5 and tinyxml2 4.0 #749

Merged
merged 5 commits into from
Jul 21, 2016
Merged

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Jul 21, 2016

FCL switched to use the smart pointers of the C++11 standard library since 0.5.0, and removed XML_NO_ERROR since 4.0.0.


This change is Reviewable

@jslee02 jslee02 added this to the DART 6.0.1 milestone Jul 21, 2016
@mxgrey
Copy link
Member

mxgrey commented Jul 21, 2016

:lgtm:


Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@mkoval
Copy link
Collaborator

mkoval commented Jul 21, 2016

You can eliminate a lot of duplicate #if checks by defining using alias:

#if FCL_VERSION_AT_LEAST(0,5,0)
template <class T> using fcl_pointer = std::shared_ptr<T>;
#else
template <class T> using fcl_pointer = boost::shared_ptr<T>;
#endif

I prefer this to the current implementation because it doesn't clutter up the implementation with a bunch of #if/#else/#endif checks.

@jslee02
Copy link
Member Author

jslee02 commented Jul 21, 2016

You can eliminate a lot of duplicate #if checks by defining using alias:

👍 As there are uses of weak_ptr as well, I would prefer fcl_shared_ptr and fcl_weak_ptr (or fcl::shared_ptr and fcl::weak_ptr) though.

@jslee02
Copy link
Member Author

jslee02 commented Jul 21, 2016

Also, it seems there are API changes in tinyxml2 as well according to the travis build log. Will create an PR for it.

@mkoval
Copy link
Collaborator

mkoval commented Jul 21, 2016

That sounds good to me. I am concerned about the code duplication, not the name of the alias. :)

@jslee02
Copy link
Member Author

jslee02 commented Jul 21, 2016

It seems tinyxml2 removed XML_NO_ERROR in version 4.0. In the last commit, I switched to use XML_SUCCESS rather than XML_NO_ERROR since they were interchangeable (they were assigned with the same value 0).

@jslee02 jslee02 changed the title Update API to support fcl 0.5.0 Update API to support fcl 0.5 and tinyxml 4.0 Jul 21, 2016
@jslee02 jslee02 merged commit d52dd29 into release-6.0 Jul 21, 2016
@jslee02 jslee02 deleted the fcl-0.5.0 branch July 22, 2016 14:36
@jslee02 jslee02 modified the milestones: DART 6.0.2, DART 6.0.1 Jul 22, 2016
@jslee02 jslee02 restored the fcl-0.5.0 branch August 25, 2016 00:54
jslee02 added a commit that referenced this pull request Aug 25, 2016
@jslee02 jslee02 deleted the fcl-0.5.0 branch August 25, 2016 01:12
jslee02 added a commit that referenced this pull request Aug 26, 2016
@jslee02 jslee02 modified the milestones: DART 6.0.1, DART 6.0.2 Oct 6, 2016
@jslee02 jslee02 changed the title Update API to support fcl 0.5 and tinyxml 4.0 Update API to support fcl 0.5 and tinyxml2 4.0 Feb 13, 2018
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.

3 participants