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

Make getParam() functions of FilterBase const #35

Merged
merged 1 commit into from
Jul 14, 2021

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented Jun 24, 2020

There's no reason for them to be non-const.

@peci1 peci1 mentioned this pull request Jun 24, 2020
@peci1 peci1 closed this Jun 24, 2020
@peci1 peci1 reopened this Jun 24, 2020
@peci1 peci1 closed this Jun 24, 2020
@peci1 peci1 reopened this Jun 24, 2020
@peci1 peci1 marked this pull request as ready for review June 24, 2020 20:48
@peci1
Copy link
Contributor Author

peci1 commented Jun 24, 2020

The proposed solution does one unnecessary copy of the XmlRpcValue for each bool/int/double just to remove the constness. It might be overcome by const_cast, but I'd strongly vote against that.

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up. I'm going to rebase and merge it into noetic.

tfoote added a commit that referenced this pull request Sep 23, 2020
@tfoote tfoote mentioned this pull request Sep 23, 2020
tfoote added a commit that referenced this pull request Sep 23, 2020
@peci1
Copy link
Contributor Author

peci1 commented Jun 29, 2021

Now that the noetic PR is merged, could this be merged to lunar-devel?

@jonbinney jonbinney merged commit 0476479 into ros:lunar-devel Jul 14, 2021
peci1 added a commit to peci1/robot_body_filter that referenced this pull request Jan 6, 2022
Allowed by ros/filters#35 (released in Melodic filters 1.8.2 (October 2021) and Noetic filters 1.9.1 (September 2021)).
peci1 added a commit to ctu-vras/ros-utils that referenced this pull request Aug 26, 2022
Allowed by ros/filters#35 (released in Melodic filters 1.8.2 (October 2021) and Noetic filters 1.9.1 (September 2021)).
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