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

[foxy] Backport #1023 #1167 #1169

Merged
merged 2 commits into from
Jun 17, 2020
Merged

[foxy] Backport #1023 #1167 #1169

merged 2 commits into from
Jun 17, 2020

Conversation

ivanpauno
Copy link
Member

This PR will be merged with --ff-only, to preserve commit hashes.
Backports #1023 #1167.

I will wait until tomorrow to merge, so all nightly jobs were ran once with these changes in.

@ivanpauno ivanpauno added the in review Waiting for review (Kanban column) label Jun 11, 2020
@ivanpauno ivanpauno self-assigned this Jun 11, 2020
@ivanpauno ivanpauno changed the base branch from master to foxy June 11, 2020 20:35
@ivanpauno
Copy link
Member Author

PR checker seems to have randomly failed.
@ros-pull-request-builder retest this please.

@jacobperron
Copy link
Member

@ros-pull-request-builder retest this please.

@jacobperron
Copy link
Member

PR checker seems to have randomly failed.

Clearing the workspace and retriggering seems to fix the issue.

@ivanpauno
Copy link
Member Author

I've edited #1023, to make it completely API compatible.

#1023 adds spin_once_impl as a protected method, thus it could participate in overload resolution if somebody was extending Executor class externally and named one of their methods spin_once_impl (extremely unlikely, but possible).
I made the method private, so it can't particiapate in overload resolution.

@jacobperron
Copy link
Member

jacobperron commented Jun 12, 2020 •
edited
Loading

I made the method private, so it can't particiapate in overload resolution.

@ivanpauno Looks okay to me. Can you trigger CI for all platforms?

@ivanpauno
Copy link
Member Author

ivanpauno commented Jun 12, 2020 •
edited
Loading

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status (known flaky tests)

@ivanpauno
Copy link
Member Author

ivanpauno commented Jun 16, 2020 •
edited
Loading

Last macOS job seem to have randomly failed, new one:

  • macOS Build Status (known flaky/failing tests)

@dirk-thomas
Copy link
Member

@ivanpauno Please mention what is being backported in the title since that will become the commit message once squash merged. The referenced ticket numbers aren't too helpful when reading just the commit message.

@ivanpauno
Copy link
Member Author

@ivanpauno Please mention what is being backported in the title since that will become the commit message once squash merged. The referenced ticket numbers aren't too helpful when reading just the commit message.

I'm going to rebase merge (actually --ff-only merge) to keep a clean history, the old commit messages will be preserved.

@ivanpauno
Copy link
Member Author

ivanpauno commented Jun 16, 2020 •
edited
Loading

@jacobperron does this look ready to be merged?

jacobperron
jacobperron approved these changes Jun 17, 2020
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM

DongheeYe and others added 2 commits June 17, 2020 09:52
@DongheeYe @ivanpauno
Fix spin_until_future_complete: check spinning value (#1023)
70e1830 
Signed-off-by: Donghee Ye <donghee.ye@samsung.com>

Make Executor::spin_once_impl private before backporting, to avoid API compatibility issues

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Check if context is valid when looping in spin_some (#1167)
c14f46e 
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno merged commit c14f46e into foxy Jun 17, 2020
@ivanpauno
Copy link
Member Author

For the record, I've rebased with foxy branch before merging without modifying code.

@ivanpauno ivanpauno deleted the ivanpauno/foxy-#1023-#1167 branch June 17, 2020 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobperron jacobperron jacobperron approved these changes

Assignees

@ivanpauno ivanpauno

Labels
in review Waiting for review (Kanban column)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ivanpauno @jacobperron @dirk-thomas @DongheeYe