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.

Sign up for GitHub

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

Jump to bottom

Reduce example names to be able to run Conda CI on Windows (gz-cmake4) #476

Merged
merged 12 commits into from
Feb 25, 2025
Merged

Reduce example names to be able to run Conda CI on Windows (gz-cmake4) #476

merged 12 commits into from
Feb 25, 2025

Conversation

j-rivero
Copy link
Contributor

🦟 Bug fix

Fixes the compilation on Windows.

Summary

Port of #463

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.

j-rivero and others added 2 commits January 29, 2025 20:29
@j-rivero
Reduce example names to be able to run Conda CI on Windows (gz-cmake2) (
9498b62 
#463)

* Finish with the massive renaming

---------
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@j-rivero
Fixes after the initial merge
9287af6 
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
@j-rivero j-rivero requested a review from scpeters as a code owner February 17, 2025 12:47
@github-actions github-actions bot added 🏛️ ionic Gazebo Ionic 🪵 jetty Gazebo Jetty labels Feb 17, 2025
@j-rivero
Port test to gz
a3d8291 
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
@j-rivero
Typo in variable name
e1493c1 
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
@j-rivero
Do not use PREFIX_PATH and INSTALL_PREFIX for the same value
50ff266 
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
@j-rivero j-rivero mentioned this pull request Feb 20, 2025
Open
iche033
iche033 approved these changes Feb 21, 2025
View reviewed changes
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

The changes look fine to me. It's unfortunate that we have to make the names less descriptive just for windows / conda. Since the changes are only in examples, I think it should be ok.

scpeters
scpeters reviewed Feb 21, 2025
View reviewed changes
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

I haven't reviewed all of this, but it looks like there's some files from a bad merge

examples/no_ign_prefix/test/CMakeLists.txt Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I think this came from a bad merge and should be removed

examples/no_ign_prefix/src/CMakeLists.txt Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I think this came from a bad merge and should be removed

examples/no_ign_prefix/include/no_ign/config.hh.in Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I think this came from a bad merge and should be removed

@j-rivero
Copy link
Contributor Author

The changes look fine to me. It's unfortunate that we have to make the names less descriptive just for windows / conda.

Indeed. An alternative is to disable the examples testing on Windows if I thought it has less value than changing the full descriptive names.

scpeters added a commit that referenced this pull request Feb 22, 2025
@scpeters
Shorten build type string in example builds
e92c41a 
Part of #476.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters mentioned this pull request Feb 22, 2025
Merged
8 tasks
@scpeters
Copy link
Member

I split out a small portion of this related to shortening the build type string into #478 to simplify review of this PR

j-rivero pushed a commit that referenced this pull request Feb 25, 2025
@scpeters
Shorten build type string in example builds (#478)
5db0bf9 
Part of #476.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@j-rivero
Merge branch 'gz-cmake4' into jrivero/port_463
dea99db 
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
@j-rivero
Remove no_ign_prefix example
fa0ddc4 
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
@j-rivero
Fix bad merge
d5240a2 
Signed-off-by: Jose Luis Rivero <jrivero@honurobotics.com>
@j-rivero
Copy link
Contributor Author

I split out a small portion of this related to shortening the build type string into #478 to simplify review of this PR

@scpeters how you want to move forward with the split of the PR technique? Currently this one is broken after the merge of #478

@scpeters
One more fix to merge
dff7c93 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Copy link
Member

I split out a small portion of this related to shortening the build type string into #478 to simplify review of this PR

@scpeters how you want to move forward with the split of the PR technique? Currently this one is broken after the merge of #478

let's see if dff7c93 fixes it

@scpeters
Copy link
Member

I'm making one final review pass

@scpeters
fix whitespace and readmes
6e2d55b 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters
scpeters reviewed Feb 25, 2025
View reviewed changes
examples/comp_depsA/CMakeLists.txt
Copy link
Member

Choose a reason for hiding this comment

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

some of these files are mixed up; this is _depsC -> _depsA

Copy link
Member

Choose a reason for hiding this comment

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

I made some changes in 85ce09d which I believe are correct but the git diff is still confused

@scpeters
revert changes to comp_deps[ABC] examples
85ce09d 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
fix a readme
b47a4a4 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters
scpeters approved these changes Feb 25, 2025
View reviewed changes
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

I manually verified the changes since the GitHub diff was confused about the renamed files

@scpeters scpeters merged commit 1c5630a into gz-cmake4 Feb 25, 2025
10 checks passed
@scpeters scpeters deleted the jrivero/port_463 branch February 25, 2025 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scpeters scpeters scpeters approved these changes

@iche033 iche033 iche033 approved these changes

Assignees
No one assigned
Labels
🏛️ ionic Gazebo Ionic 🪵 jetty Gazebo Jetty
Projects
Core development
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@j-rivero @scpeters @iche033