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

Remove ign-tools from CMakeLists.txt. Not used #56

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

j-rivero
Copy link
Contributor

🦟 Bug fix

Summary

Code is looking for ign-tools while I can not find any place where it is used.

Checklist

Note to maintainers: Remember to use Squash-Merge

@github-actions github-actions bot added 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome labels Aug 18, 2021
@codecov
Copy link

codecov bot commented Aug 18, 2021

Codecov Report

Merging #56 (d7de400) into ign-plugin1 (ccf671d) will decrease coverage by 4.10%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-plugin1      #56      +/-   ##
===============================================
- Coverage        99.82%   95.71%   -4.11%     
===============================================
  Files               15       15              
  Lines              584      584              
===============================================
- Hits               583      559      -24     
- Misses               1       25      +24     
Impacted Files Coverage Δ
loader/src/ign.cc 0.00% <0.00%> (-96.00%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccf671d...d7de400. Read the comment docs.

@ahcorde ahcorde merged commit 111ba95 into ign-plugin1 Aug 18, 2021
@ahcorde ahcorde deleted the remove_ign_tools branch August 18, 2021 20:57
@traversaro
Copy link

That line was added in https://github.com/ignitionrobotics/ign-plugin/pull/32/files, and it seems that it was used at least in https://github.com/ignitionrobotics/ign-plugin/blob/ign-plugin1/loader/CMakeLists.txt#L7 to decide if to include src/ign_TEST.cc in the compiled tests.

scpeters added a commit to scpeters/ign-plugin that referenced this pull request Aug 19, 2021
The find_package(ignition-tools) call was removed in gazebosim#56,
but this disabled UNIT_ign_TEST. Search for the ign
program directly and adjust test logic accordingly.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
Copy link
Member

That line was added in https://github.com/ignitionrobotics/ign-plugin/pull/32/files, and it seems that it was used at least in https://github.com/ignitionrobotics/ign-plugin/blob/ign-plugin1/loader/CMakeLists.txt#L7 to decide if to include src/ign_TEST.cc in the compiled tests.

see #57

ahcorde pushed a commit that referenced this pull request Aug 19, 2021
The find_package(ignition-tools) call was removed in #56,
but this disabled UNIT_ign_TEST. Search for the ign
program directly and adjust test logic accordingly.

Signed-off-by: Steve Peters <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants