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.

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

fix: linter tests #251

Merged
merged 5 commits into from
May 20, 2022
Merged

fix: linter tests #251

merged 5 commits into from
May 20, 2022

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented May 18, 2022

🦟 Bug fix

Fixes #

Summary

fix some linter tests

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Daisuke Nishimatsu added 3 commits May 19, 2022 06:05
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 requested a review from chapulina as a code owner May 18, 2022 21:16
@wep21 wep21 changed the title Fix/linter tests fix: linter tests May 18, 2022
ros_ign_gazebo/CMakeLists.txt Outdated Show resolved Hide resolved
@wep21
Copy link
Contributor Author

wep21 commented May 18, 2022

@chapulina I appreciate it if you do bloom-release to rolling/humble distribution after merging this PR.

Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I pushed 94da01d with 2 changes:

  • Restore testing repo for CI
  • Instead of suppressing the ament warnings, fix the include headers. This is finally our chance to group the Gazebo and ROS C++ headers together 😄

I noticed there's a test failure both locally for me and on CI. I'm not sure if it's something we're doing wrong or some regression upstream. I'll merge with that failure for now, since this PR is about linters.

[4.654s] 1: [test_ros_subscriber-3] cannot publish data, at ./src/rmw_publish.cpp:62 during '__function__'
[4.655s] 1: [test_ros_subscriber-3] [ERROR] [1653006329.567666397] [ros_subscriber.rclcpp]: Error in destruction of rcl subscription handle: Failed to delete datareader, at ./src/subscription.cpp:52, at ./src/rcl/subscription.c:183
[4.655s] 1: [test_ros_subscriber-3] cannot publish data, at ./src/rmw_publish.cpp:62 during '__function__'
[4.655s] 1: [test_ros_subscriber-3] Fail in delete datareader, at ./src/rmw_service.cpp:103 during '__function__'

.github/workflows/build-and-test.sh Outdated Show resolved Hide resolved
@chapulina chapulina merged commit 4dcf207 into gazebosim:ros2 May 20, 2022
@wep21 wep21 deleted the fix/linter_tests branch May 20, 2022 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants