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

Move test dependencies into BUILD_TESTING to avoid build error #241

Merged
merged 2 commits into from
Aug 16, 2018

Conversation

gaoethan
Copy link
Contributor

Fix the issue in ros2/rosdistro#260 due to some test dependencies that are find_package'd outside of a BUILD_TESTING conditional.

Copy link
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

This code is duplicated so it should be removed from this file rather than moved inside the BUILD_TESTING block.

As most linter tests are failing (see log from ros2/rosdistro#260 (comment)), we should either fix the test failures or disable the tests will not be fixed (if for example we want to keep the ROS 1 style to keep the ros2 branch as close to upstream as possible)

if(BUILD_TESTING)
find_package(ament_lint_auto REQUIRED)
ament_lint_auto_find_test_dependencies()
Copy link
Contributor

Choose a reason for hiding this comment

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

This code is already present in the test/CMakeLists.txt file. So having it here as well is redundant, it would be better to remove it.

@gaoethan gaoethan merged commit 555c377 into ros-perception:ros2 Aug 16, 2018
@gaoethan
Copy link
Contributor Author

It's removed accordingly, thanks @mikaelarguedas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants