Revert smart pointer type change in PointCloud and deprecate getMapping()
#3563
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kunaltyagi
requested changes
Jan 17, 2020
common/include/pcl/point_cloud.h
Outdated
| @@ -603,17 +606,17 @@ namespace pcl | |||
|
|
|||
| protected: | |||
| // This is motivated by ROS integration. Users should not need to access mapping_. | |||
| std::shared_ptr<MsgFieldMap> mapping_; | |||
| [[deprecated("rewrite your code to avoid using it")]] boost::shared_ptr<MsgFieldMap> mapping_; | |||
There was a problem hiding this comment.
Maybe reword the message to include what to do instead. Like Use pcl::createMapping(vector<pcl::PCLPointField>, pcl::MsgFieldMap&) instead
There was a problem hiding this comment.
I also had this thought but was not sure since it's not a true replacement. But since you also think this way, I'll update the message.
There was a problem hiding this comment.
It's not public, but since there was a public usage, people (maintainers) who compile from source (and check warnings 😅 ) will know
There was a problem hiding this comment.
Well yeah, sounds a bit presumptuous, but what else can I say :D Here it's not about how to create a mapping, but rather about not using PCL's protected space to store your stuff.
Even though pcl::detail::getMapping() is not officially a public API, it is used outside of PCL (in pcl_ros). Changing the smart pointer type from boost to std broke that code. This commit partially reverses the changes from PointCloudLibrary#3474, unbreaking pcl_ros. It also properly deprecates the function so that other (unknown to us) clients have time to update their code.
taketwo
force-pushed
the
deprecate-get-mapping
branch
from
655d8b0
to
794c586
Compare
kunaltyagi
approved these changes
Jan 17, 2020
taketwo
added
changelog: deprecation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Even though
pcl::detail::getMapping()is not officially a public API, it is used outside of PCL (inpcl_ros). Changing the smart pointer type from boost to std broke that code. (More information in #2792.)This commit partially reverses the changes from #3474, unbreaking
pcl_ros. It also properly deprecates the function so that other (unknown to us) clients have time to update their code.