Skip to content

[dashing backport] Type conversions fixes (#901)#1209

Merged
hidmic merged 2 commits intodashingfrom
hidmic/dashing/backport-901
Sep 17, 2020
Merged

[dashing backport] Type conversions fixes (#901)#1209
hidmic merged 2 commits intodashingfrom
hidmic/dashing/backport-901

Conversation

@hidmic
Copy link
Copy Markdown
Contributor

@hidmic hidmic commented Jul 1, 2020

  • Fix type conversions

Signed-off-by: Monika Idzik monika.idzik@apex.ai

  • Add static_casts

Signed-off-by: Monika Idzik monika.idzik@apex.ai

  • Address PR comments

Signed-off-by: Monika Idzik monika.idzik@apex.ai

  • Remove one time use variable

Signed-off-by: Monika Idzik monika.idzik@apex.ai

@hidmic hidmic mentioned this pull request Jul 1, 2020
Merged
@hidmic
Type conversions fixes (#901)
0a2cb6c 
* Fix type conversions

Signed-off-by: Monika Idzik <monika.idzik@apex.ai>

* Add static_casts

Signed-off-by: Monika Idzik <monika.idzik@apex.ai>

* Address PR comments

Signed-off-by: Monika Idzik <monika.idzik@apex.ai>

* Remove one time use variable

Signed-off-by: Monika Idzik <monika.idzik@apex.ai>
@hidmic hidmic force-pushed the hidmic/dashing/backport-901 branch from 014d117 to 0a2cb6c Compare July 1, 2020 15:52
@hidmic
Please cpplint
3f0f031 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@nuclearsandwich
Copy link
Copy Markdown
Member

nuclearsandwich commented Jul 6, 2020

I'm hopefully fixing a few more CI issues before running CI and approving this change myself. I may combine it with other rclcpp backport PRs into an omnibus for CI.

@ivanpauno
Copy link
Copy Markdown
Member

ivanpauno commented Jul 21, 2020

@nuclearsandwich is something else pending, or can CI be run here?

@nuclearsandwich
Copy link
Copy Markdown
Member

nuclearsandwich commented Jul 23, 2020

@nuclearsandwich is something else pending, or can CI be run here?

Nope CI is running, this just wasn't on the Dashing P7 board so I lost track of it.

@nuclearsandwich
Copy link
Copy Markdown
Member

nuclearsandwich commented Jul 23, 2020

CI up to test_rclcpp running test_rclcpp and rclcpp tests. Since there is linter-drift between CI linter versions and Dashing I generally disable linter tests for full Dashing CI runs but I've left them running here. Any linter issues which are not from the changes in this backport can be disregarded.

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

nuclearsandwich
nuclearsandwich approved these changes Jul 23, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

LGTM with acceptable CI

@hidmic
Copy link
Copy Markdown
Contributor Author

hidmic commented Aug 10, 2020 •
edited
Loading

CI test failures on MacOS and Windows seem completely unrelated.

Re-running CI against stable dashing just in case:

  • macOS Build Status
  • Windows Build Status

@ivanpauno
Copy link
Copy Markdown
Member

ivanpauno commented Aug 27, 2020

@hidmic is this ready to be merged?

@hidmic
Copy link
Copy Markdown
Contributor Author

hidmic commented Sep 17, 2020

Indeed is!

@hidmic hidmic merged commit ab24842 into dashing Sep 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the hidmic/dashing/backport-901 branch September 17, 2020 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nuclearsandwich nuclearsandwich nuclearsandwich approved these changes

@ivanpauno ivanpauno ivanpauno approved these changes

Assignees

@hidmic hidmic

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hidmic @nuclearsandwich @ivanpauno