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.

Sign up for GitHub

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

Jump to bottom

Initialize the quaternion in the tf2_geometry_msgs unit tests #25

Merged
merged 1 commit into from
Jun 19, 2017
Merged

Initialize the quaternion in the tf2_geometry_msgs unit tests #25

merged 1 commit into from
Jun 19, 2017

Conversation

tfoote
Copy link
Contributor

@tfoote tfoote commented Jun 19, 2017

Connects to #24

  • Linux Build Status
  • Linux aarch64 Build Status
  • OSX Build Status
  • Windows Build Status

@tfoote tfoote added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jun 19, 2017
clalancette
clalancette approved these changes Jun 19, 2017
View reviewed changes
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Oh yeah, absolutely. This has bitten me several times.

mikaelarguedas
mikaelarguedas approved these changes Jun 19, 2017
View reviewed changes
Copy link
Member

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

lgtm, even if we dont do it by default (for performance reasons) we should consider providing a helper function giving zero initialized messages in C and C++...

@tfoote tfoote merged commit 0fa3557 into ros2 Jun 19, 2017
@tfoote tfoote removed the in review Waiting for review (Kanban column) label Jun 19, 2017
@tfoote tfoote deleted the initialize_quaternion branch June 19, 2017 22:20
@clalancette
Copy link
Contributor

Honestly, I think we should discuss making the default zero-initialized, and providing a way to not do that for people who are really performance-conscious. This kind of problem is sneaky and hard to see, and is going to affect everyone developing on ROS2.

@tfoote tfoote mentioned this pull request Jun 20, 2017
Closed
@tfoote
Copy link
Contributor Author

tfoote commented Jun 20, 2017

@clalancette @mikaelarguedas good idea. ticketed ros2/ros2#367

@tfoote tfoote mentioned this pull request Jun 23, 2017
Merged
@clalancette clalancette mentioned this pull request Aug 25, 2017
Closed
11 tasks
@clalancette clalancette mentioned this pull request Sep 6, 2017
Closed
3 tasks
@clalancette clalancette mentioned this pull request Apr 14, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

@mikaelarguedas mikaelarguedas mikaelarguedas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tfoote @clalancette @mikaelarguedas