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 tools in favor of make codecheck #211

Merged
merged 5 commits into from
May 6, 2021
Merged

Conversation

chapulina
Copy link
Contributor

Summary

Removing:

In favor of:

Test it

Check out CI results

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

@scpeters
Copy link
Member

we still need tools/check_test_ran.py

Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

we still need tools/check_test_ran.py

Restored in 1c4e12d. Do you think this is something reasonable to move to ign-cmake?

@chapulina
Copy link
Contributor Author

something reasonable to move to ign-cmake?

Probably yes, it's used by it: https:/ignitionrobotics/ign-cmake/blob/17b7058ff5c9de2f6f396d1a000dd0366481d621/cmake/IgnUtils.cmake#L1721

@scpeters
Copy link
Member

something reasonable to move to ign-cmake?

Probably yes, it's used by it: https:/ignitionrobotics/ign-cmake/blob/17b7058ff5c9de2f6f396d1a000dd0366481d621/cmake/IgnUtils.cmake#L1721

since we aren't using QTest anymore (gazebosim/gz-gui#198 (comment)), I think it can be moved to ign-cmake. There are a few variants of check_test_ran.py in our packages, so we should make sure to get the one that uses an informative classname:

@chapulina
Copy link
Contributor Author

It looks like we don't even need gazebo-tooling/release-tools#455, the job already ignores a missing code_check file:

sh: 0: Can't open tools/code_check.sh
+ true

@chapulina
Copy link
Contributor Author

I think it can be moved to ign-cmake

@chapulina chapulina added needs upstream release Blocked by a release of an upstream library and removed needs upstream release Blocked by a release of an upstream library labels May 1, 2021
@chapulina chapulina enabled auto-merge (squash) May 5, 2021 23:50
@codecov
Copy link

codecov bot commented May 6, 2021

Codecov Report

Merging #211 (6bf2f4d) into ign-math6 (f6e367f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           ign-math6     #211   +/-   ##
==========================================
  Coverage      99.21%   99.21%           
==========================================
  Files             65       65           
  Lines           6089     6089           
==========================================
  Hits            6041     6041           
  Misses            48       48           

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 f6e367f...6bf2f4d. Read the comment docs.

@chapulina chapulina merged commit 9519ce7 into ign-math6 May 6, 2021
@chapulina chapulina deleted the chapuilna/code_check branch May 6, 2021 09:15
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.

2 participants