Skip to content

Conversation

@sloretz
Copy link
Contributor

@sloretz sloretz commented Jun 24, 2020

@sloretz
Copy link
Contributor Author

sloretz commented Jun 24, 2020

CI (building up to and testing only test_rmw_implementation)

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

@sloretz sloretz self-assigned this Jun 24, 2020
@sloretz sloretz added the bug Something isn't working label Jun 24, 2020
@dirk-thomas
Copy link
Member

Shouldn't it be a test_depend instead?

@sloretz
Copy link
Contributor Author

sloretz commented Jun 24, 2020

Shouldn't it be a test_depend instead?

I think either works. I picked buildtool_depend to match <buildtool_depend>rmw_implementation_cmake</buildtool_depend> which could also be a test_depend for the same reason.

@sloretz
Copy link
Contributor Author

sloretz commented Jun 24, 2020

Shouldn't it be a test_depend instead?

Actually, it brings up an interesting case with the package.xml when cross-compiling. Does <test_depend> mean the dependency is needed on the target machine or the buildtool machine in order to run tests? Maybe there should be a <buildtool_test_depend> for dependencies needed only on the buildtool machine to run tests..

@sloretz
Copy link
Contributor Author

sloretz commented Jun 24, 2020

Maybe there should be a <buildtool_test_depend> for dependencies needed only on the buildtool machine to run tests..

Ah nevermind, REP 149 already covers this:

When building with testing enabled, the <test_depend> packages are available for configuring and building the tests as well as running them. Generated Debian packages are built without the unit tests or their dependencies.

@sloretz
Copy link
Contributor Author

sloretz commented Jun 24, 2020

Shouldn't it be a test_depend instead?

@dirk-thomas rmw_implementation_cmake and ament_cmake_gtest made test_depend in f7e0563

@sloretz
Copy link
Contributor Author

sloretz commented Jun 24, 2020

CI (building and testing `test_rmw_implementation only)

  • Linux only as I'll assume a package.xml only change won't fail on other platforms if it passes on Linux Build Status

@sloretz sloretz requested a review from dirk-thomas June 24, 2020 00:39
@sloretz sloretz merged commit ff1e864 into master Jun 24, 2020
@sloretz sloretz deleted the sloretz/depend_ament_cmake_gtest branch June 24, 2020 01:00
@hidmic
Copy link
Contributor

hidmic commented Jun 24, 2020

Uh, I thought I had added the dependency. Oh well, thanks for the fix @sloretz !!

ahcorde pushed a commit that referenced this pull request Oct 9, 2020
* Add dpendency on ament_cmake_gtest

Signed-off-by: Shane Loretz <[email protected]>

* Make buildtool_depends test_depends

Signed-off-by: Shane Loretz <[email protected]>
ahcorde pushed a commit that referenced this pull request Oct 21, 2020
* Add dpendency on ament_cmake_gtest

Signed-off-by: Shane Loretz <[email protected]>

* Make buildtool_depends test_depends

Signed-off-by: Shane Loretz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants