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

Rename MultiSphereShape to MultiSphereConvexHullShape #865

Merged
merged 2 commits into from
Mar 31, 2017

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Mar 30, 2017

This PR renames MultiSphereShape to MultiSphereConvexHullShape as suggested in this comment.

@jslee02 jslee02 added this to the DART 6.2.0 milestone Mar 30, 2017
@@ -8,6 +8,8 @@ dart_add_core_sources(${srcs} ${detail_srcs})

# Generate header for this namespace
dart_get_filename_components(header_names "dynamics headers" ${hdrs})
# TODO: remove below line once the files are completely removed.
list(REMOVE_ITEM header_names "MultiSphereShape.hpp")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Member Author

@jslee02 jslee02 Mar 30, 2017

Choose a reason for hiding this comment

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

This excludes MultiSphereShape.hpp from the generated header dart/dynamics/dynamics.hpp so that it suppresses the warning of including the deprecated header.


};

using MultiSphereShape = MultiSphereConvexHullShape;
Copy link
Collaborator

@mkoval mkoval Mar 31, 2017

Choose a reason for hiding this comment

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

I think we can also mark this using alias as deprecated:

Indicates that the use of the name or entity declared with this attribute is allowed, but discouraged for some reason. This attribute is allowed in declarations of classes, typedef-names, variables, non-static data members, functions, enumerations, and template specializations. A name declared non-deprecated may be redeclared deprecated. A name declared deprecated cannot be un-deprecated by redeclaring it without this attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

We probably should stay with DART_DEPRECATED(...) until DART uses C++14 because [[deprecated]] is available since C++14.

By the way, maybe I'm missing something here, but it seems DART_DEPRECATED(...) is not available on typedef using using:

DART_DEPRECATED(6.2)
using MultiSphereShape = MultiSphereConvexHullShape; // can't compile

DART_DEPRECATED(6.2)
typedef MultiSphereConvexHullShape MultiSphereShape; // okay

Copy link
Collaborator

Choose a reason for hiding this comment

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

That might be true. That webpage only mentions typedef. While I generally favor using directives to typedefs, I am slightly in favor of using DART_DEPRECATED(6.2) typedef here since this is a temporary workaround that will only last until the next major revision.

@jslee02 jslee02 merged commit f4cbcc1 into master Mar 31, 2017
@jslee02 jslee02 deleted the feature/rename_multisphereshape branch March 31, 2017 17:37
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