Skip to content

Build examples from CMake rather than executable#482

Merged
mjcarroll merged 15 commits intogz-math7from
mjcarroll/examples_from_cmake
Jul 17, 2023
Merged

Build examples from CMake rather than executable#482
mjcarroll merged 15 commits intogz-math7from
mjcarroll/examples_from_cmake

Conversation

@mjcarroll
Copy link
Copy Markdown

@mjcarroll mjcarroll commented Aug 10, 2022

🦟 Bug fix

Summary

Part of gazebosim/gz-cmake#301

Test by building gz-math as normal and then running the "EXAMPLES_Configure_TEST" and "EXAMPLES_Build_TEST" via ctest.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@github-actions github-actions bot added the 🌱 garden Ignition Garden label Aug 10, 2022
Build examples from CMake rather than executable
af66e6e 
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll force-pushed the mjcarroll/examples_from_cmake branch from e6b9f87 to af66e6e Compare August 10, 2022 18:44
@mjcarroll mjcarroll mentioned this pull request Aug 10, 2022
Merged
9 tasks
@chapulina chapulina added bug Something isn't working tests Broken or missing tests / testing infra Windows Windows support labels Aug 10, 2022
@mjcarroll
Copy link
Copy Markdown
Author

mjcarroll commented Aug 10, 2022

Tested locally on Windows with:

colcon build --packages-up-to gz-math7 --merge-install
Summary: 3 packages finished [5 min 20s]

colcon test --packages-select gz-math7 --merge-install
Summary: 1 package finished [31.9s]

I also found a neat side effect, this lets us additionally turn on ninja on windows:

colcon build --packages-up-to gz-math7 --merge-install
Summary: 3 packages finished [1 min 1 s]

colcon test --packages-select gz-math7 --merge-install
Summary: 1 package finished [22.1s]

@ahcorde ahcorde added the needs upstream release Blocked by a release of an upstream library label Aug 15, 2022
@mjcarroll mjcarroll requested a review from azeey September 27, 2022 17:48
@mjcarroll
Copy link
Copy Markdown
Author

mjcarroll commented May 24, 2023

@osrf-jenkins retest this please

1 similar comment
@mjcarroll
Copy link
Copy Markdown
Author

mjcarroll commented May 26, 2023

@osrf-jenkins retest this please

Michael Carroll and others added 2 commits July 14, 2023 09:52
ahcorde
ahcorde requested changes Jul 17, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Changes LGTM, but CI is ugly, all python tests are failing

Michael Carroll added 3 commits July 17, 2023 20:10
Make python module for use in tests
225e75d 
Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
Create directory before touching
b158cce 
Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
Fix ruby as well
cedd4a4 
Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
@mjcarroll mjcarroll requested a review from ahcorde July 17, 2023 20:53
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 17, 2023

Codecov Report

Merging #482 (482ad67) into gz-math7 (cf5d7f3) will decrease coverage by 5.36%.
The diff coverage is 100.00%.

❗ Current head 482ad67 differs from pull request most recent head cedd4a4. Consider uploading reports for the commit cedd4a4 to get more accurate results

@@             Coverage Diff              @@
##           gz-math7     #482      +/-   ##
============================================
- Coverage     99.70%   94.34%   -5.36%     
============================================
  Files            77      144      +67     
  Lines          7049     9660    +2611     
============================================
+ Hits           7028     9114    +2086     
- Misses           21      546     +525
Impacted Files Coverage Δ
include/gz/math/Helpers.hh 100.00% <100.00%> (ø)
include/gz/math/Polynomial3.hh 100.00% <100.00%> (ø)

... and 82 files with indirect coverage changes

@mjcarroll mjcarroll merged commit f64cb8c into gz-math7 Jul 17, 2023
@mjcarroll mjcarroll deleted the mjcarroll/examples_from_cmake branch July 17, 2023 21:55
This was referenced Jul 19, 2023
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azeey azeey azeey approved these changes

@ahcorde ahcorde ahcorde approved these changes

@scpeters scpeters Awaiting requested review from scpeters scpeters is a code owner

@adityapande-1995 adityapande-1995 Awaiting requested review from adityapande-1995 adityapande-1995 is a code owner

Assignees

No one assigned

Labels

bug Something isn't working 🌱 garden Ignition Garden needs upstream release Blocked by a release of an upstream library tests Broken or missing tests / testing infra Windows Windows support

Projects

Core development
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mjcarroll @azeey @ahcorde @chapulina @osrf-triage