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

Refactor ros2doctor hello verb. #521

Merged
merged 3 commits into from
Jun 19, 2020
Merged

Refactor ros2doctor hello verb. #521

merged 3 commits into from
Jun 19, 2020

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented May 29, 2020 •
edited
Loading

For better resource management and to avoid races in tests. Hopefully this resolves race in Windows CI.

@hidmic hidmic mentioned this pull request May 29, 2020
Merged
@hidmic
Refactor ros2doctor hello verb.
d884dd7 
For better resource management and to avoid races in tests.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic force-pushed the hidmic/refactor-ros2doctor-hello branch from 909f7e3 to d884dd7 Compare May 29, 2020 20:41
@jacobperron jacobperron added this to In progress in Foxy Patch Release 1 via automation Jun 1, 2020
@hidmic
Copy link
Contributor Author

hidmic commented Jun 8, 2020

@claireyywang @jacobperron ping

@hidmic
Copy link
Contributor Author

hidmic commented Jun 8, 2020 •
edited
Loading

CI up to ros2doctor:

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

claireyywang
claireyywang approved these changes Jun 9, 2020
View reviewed changes
Copy link
Contributor

@claireyywang claireyywang left a comment

Choose a reason for hiding this comment

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

LGTM

@hidmic
Explicitly send to localhost.
25f1ec7 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic requested a review from ivanpauno June 16, 2020 19:02
ivanpauno
ivanpauno approved these changes Jun 19, 2020
View reviewed changes
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

I don't have much context about the previous code, but changes lgtm

ros2doctor/ros2doctor/verb/hello.py Outdated Show resolved Hide resolved
@hidmic
Explicitly destroy pub/sub.
4836e4b 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Contributor Author

hidmic commented Jun 19, 2020

Alright, thank you all for the reviews !

@hidmic hidmic merged commit d109a69 into master Jun 19, 2020
@delete-merged-branch delete-merged-branch bot deleted the hidmic/refactor-ros2doctor-hello branch June 19, 2020 19:33
@jacobperron jacobperron removed this from In progress in Foxy Patch Release 1 Jun 24, 2020
craigh92 pushed a commit to craigh92/ros2cli that referenced this pull request Jul 17, 2020
@hidmic
Refactor ros2doctor hello verb. (ros2#521)
66d7e96 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Craig Hickman <craig.hickman@ukaea.uk>
@sloretz sloretz mentioned this pull request Dec 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claireyywang claireyywang claireyywang approved these changes

@ivanpauno ivanpauno ivanpauno approved these changes

@jacobperron jacobperron Awaiting requested review from jacobperron

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@hidmic @claireyywang @ivanpauno