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

Minor cleanup and fixes #336

Merged
merged 3 commits into from
Jul 25, 2018
Merged

Minor cleanup and fixes #336

merged 3 commits into from
Jul 25, 2018

Conversation

Martin-Idel-SI
Copy link
Contributor

Fixes #334
Cleanup in rviz_visual_testing_framework: Add -Werror and fix warnings
Cleanup in CMakeLists: Additional environment not necessary (potentially harmful):

  • With ament, it was possible to build the library and then test it without having sourced it. This would lead to missing dlls on Windows.
  • Now, that colcon is the standard, anybody will first build the whole package, source the folder and then test it.

CI:

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

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Jul 12, 2018
@Martin-Idel-SI
Copy link
Contributor Author

Martin-Idel-SI commented Jul 13, 2018 •
edited

It seems that uncrustify cannot be found on Linux, aarch and Windows. I guess this is a problem on your machines with the recent update?

@wjwwood
Copy link
Member

wjwwood commented Jul 13, 2018

@mikaelarguedas any idea on this? I guess @Martin-Idel-SI you might need to make sure you're using an up-to-date ros2.repos file as the foundation of your modified one?

@mikaelarguedas
Copy link
Member

Yes, you need to update your repos file: (similar to https://github.com/ros2/ros2/pull/527/files)
Or (if you're targetting bouncy and not master) use the bouncy branch of the various repos.

@Martin-Idel-SI
Copy link
Contributor Author

Martin-Idel-SI commented Jul 16, 2018 •
edited

It's weird, because I thought I did take a new one. Anyway, new again:

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

wjwwood
wjwwood approved these changes Jul 25, 2018
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm

@wjwwood wjwwood merged commit 8ae0604 into ros2:ros2 Jul 25, 2018
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label Jul 25, 2018
@Martin-Idel-SI Martin-Idel-SI deleted the bugfix/additional_fixes_after_uncrustify branch August 9, 2018 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjwwood wjwwood wjwwood 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.

warning on macOS CI (undetected by Jenkins)
5 participants
@Martin-Idel-SI @wjwwood @mikaelarguedas @dirk-thomas @botteroa-si