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

Complete publisher/subscription QoS query API test coverage. #120

Merged
merged 2 commits into from
Sep 7, 2020
Merged

Complete publisher/subscription QoS query API test coverage. #120

merged 2 commits into from
Sep 7, 2020

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Aug 20, 2020

@hidmic
Complete publisher/subscription QoS query API test coverage.
adb2e7f 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic requested a review from Blast545 August 20, 2020 15:23
Blast545
Blast545 approved these changes Aug 20, 2020
View reviewed changes
Copy link
Contributor

@Blast545 Blast545 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. Check PR job before merging, seems to be a problem there.

@hidmic
Drop no allocation requirement from QoS query API tests.
3b32d71 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Contributor Author

hidmic commented Aug 24, 2020

Indeed, had to relax allocation requirements.

@hidmic
Copy link
Contributor Author

hidmic commented Aug 24, 2020

CI up to test_rmw_implementation:

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

PR job won't pass until ros2/rmw_fastrtps#425 gets merged and rmw_fastrtps is re-released into rolling.

@hidmic
Copy link
Contributor Author

hidmic commented Sep 1, 2020

@ros-pull-request-builder retest this please

Blast545
Blast545 approved these changes Sep 2, 2020
View reviewed changes
Copy link
Contributor

@Blast545 Blast545 left a comment

Choose a reason for hiding this comment

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

LGTM, might be a good idea to re run CI if the PR job doesn't pass

@hidmic
Copy link
Contributor Author

hidmic commented Sep 2, 2020

@ros-pull-request-builder retest this please

@hidmic
Copy link
Contributor Author

hidmic commented Sep 2, 2020

Argh, rosdep timed out fetching sources.

@ros-pull-request-builder retest this please

@hidmic
Copy link
Contributor Author

hidmic commented Sep 3, 2020

Let's try again:

@ros-pull-request-builder retest this please

@hidmic
Copy link
Contributor Author

hidmic commented Sep 7, 2020

Alright, all CI's green. Going in!

@hidmic hidmic merged commit fbe44b1 into master Sep 7, 2020
@delete-merged-branch delete-merged-branch bot deleted the hidmic/rmw-pub-sub-qos-tests branch September 7, 2020 15:24
ahcorde pushed a commit that referenced this pull request Oct 9, 2020
@hidmic @ahcorde
Complete publisher/subscription QoS query API test coverage. (#120)
72fdbca 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
ahcorde pushed a commit that referenced this pull request Oct 21, 2020
@hidmic @ahcorde
Complete publisher/subscription QoS query API test coverage. (#120)
20e2a43 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Blast545 Blast545 Blast545 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.
2 participants
@hidmic @Blast545