Skip to content

Fixed in Exception constructor in tf2_ros#267

Merged
ahcorde merged 1 commit intoros2from
ahcorde/fix/tf2
May 19, 2020
Merged

Fixed in Exception constructor in tf2_ros#267
ahcorde merged 1 commit intoros2from
ahcorde/fix/tf2

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented May 18, 2020

This PR builds on top of this other PR #258. Fix some issues in tf2_ros

Signed-off-by: ahcorde ahcorde@gmail.com

@ahcorde
Fixed in Exception constructor tf2_ros
cc857cf 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde added the enhancement New feature or request label May 18, 2020
@ahcorde ahcorde requested review from clalancette and tfoote May 18, 2020 07:44
@ahcorde ahcorde self-assigned this May 18, 2020
@ahcorde ahcorde mentioned this pull request May 18, 2020
Merged
@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented May 18, 2020

The code looks OK to me, but can you explain why this is necessary? The code in question succeeds in CI, so I'm just curious why we need it now.

hidmic
hidmic approved these changes May 18, 2020
View reviewed changes
Copy link
Copy Markdown

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM!

@clalancette I suspect this patch makes use of std::make_exception_ptr in a simpler way, without relying on non-explicit constructors for the given exception type.

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented May 19, 2020

@clalancette I suspect this patch makes use of std::make_exception_ptr in a simpler way, without relying on non-explicit constructors for the given exception type.

Yeah, understood. I'm just not sure what is different about the Fpr environment vs. our CI environment, and if that is something we should look into.

Regardless, this is an improvement over the status-quo, so I'll approve.

clalancette
clalancette approved these changes May 19, 2020
View reviewed changes
Copy link
Copy Markdown
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.

Looks good to me, please also run CI on it.

@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented May 19, 2020

Running up-to geometry2. Test only for tf2_ros

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

@ahcorde ahcorde merged commit 737a362 into ros2 May 19, 2020
@delete-merged-branch delete-merged-branch bot deleted the ahcorde/fix/tf2 branch May 19, 2020 17:40
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

@hidmic hidmic hidmic approved these changes

@tfoote tfoote Awaiting requested review from tfoote

Assignees

@ahcorde ahcorde

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ahcorde @clalancette @hidmic