-
Notifications
You must be signed in to change notification settings - Fork 138
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
[ros2] Add default ign_gazebo version #145
Conversation
I added to the checklist another pr which this one depends on (gazebosim/gz-tools#44) Is a unit test desired? (maybe we could test it by using specific configurations with the ci?) |
@chapulina I've opened the PR to the wrong branch, could you please move it to the ros2 branch please? 🤦 edit: fixed |
368734b
to
05fcc87
Compare
return ign_args | ||
|
||
|
||
def __get_ign_gazebo_default_version(): # this will fail if the package build was performed with '--merge-install' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To the reviewer:
At least locally I couldn't find the package when my workspace was build with --merge-install
flag, apparently the overlay does not work with such configuration (also noticed that the package manifest was not installed).
05fcc87
to
cf34067
Compare
dc2cba0
to
dc639c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mind to fix the conflicts with the main branch ?
Done |
Signed-off-by: Caio Amaral <[email protected]> Minor Fix
5b8aeaa
to
e37b143
Compare
Friendly ping. I think this one is good to go now that gazebosim/gz-tools#44 is merged |
Thanks, @caioaamaral , we'll take a look soon. Note that we'll need an |
I'm getting an error on launch, I tried compiling with
|
@caioaamaral , are you still working on this PR? |
Sorry for the delay. I'll have a look by the end of this week |
Hi @caioaamaral , I'm closing this due to inactivity, but feel free to reopen if you start working on it again. Thanks! |
🦟 Bug fix
Fixes #133
Summary
This change uses as the default version the dependency resolved by the package manifest (which uses ignition-gazebo3 for the foxy distro). This is accomplished by appending the flag
--force-version
(only when it's not already set) to theign_args
Checklist
Signed all commits for DCO
Added tests
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
Depends on: Relax version verification gz-tools#44 (to allow passing the major version only)
Note to maintainers: Remember to use Squash-Merge