-
Notifications
You must be signed in to change notification settings - Fork 47
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
ign -> gz Namespace Migration : gz-msgs #252
Conversation
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
84839de
to
5a2d05f
Compare
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
d675836
to
7955b00
Compare
Signed-off-by: methylDragon <[email protected]>
7955b00
to
12033d2
Compare
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Migrating ignition.msgs -> gz.msgs leads to a change in index for splitting any input string. This fixes a failing test. Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
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.
I believe we should be able to tick-tock headers and namespaces. The only thing we haven't found a way of tick-tocking is custom message generation. Most Gazebo libraries and external users don't create custom messages though, so I think it's worth at least tick-tocking headers and namespaces.
I tried these small changes on top of your branches:
- namespace_migration...chapulina/namespace_migration
- Add
ignition
redirection headers for the generated protobuf (only did one msg as a POC) - Add namespace alias with pragma warning
- Add
- gazebosim/gz-transport@namespace_migration...chapulina/namespace_migration
- Changed one place on
ign-transport
to include the old header and use the old namespace. - The test compiles with a warning, and passes.
- Changed one place on
What do you think?
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
Migration updated accordingly: e99f08a |
Oh yeah, and because so many files are being moved, we'll need a clean commit history to rebase-merge here. 🙃 |
Signed-off-by: methylDragon <[email protected]>
The files being moved here are the redirection headers added in the header migration, do we need to preserve the history for those (there's no history aside from that single commit)? |
Signed-off-by: methylDragon <[email protected]>
7245abe
to
f536320
Compare
Signed-off-by: methylDragon <[email protected]> Co-authored-by: Louise Poubel <[email protected]>
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.
The files being moved here are the redirection headers added in the header migration, do we need to preserve the history for those (there's no history aside from that single commit)?
Ah fair point, we can squash merge this PR then!
LGTM with 🟢 CI
Signed-off-by: Louise Poubel <[email protected]>
See gazebo-tooling/release-tools#711
Note: The
include/gz/msgs
redirection headers point toignition
and use theignition
namespace, that's why they need theconfig.hh
to support the namespace remaps in downstream packages.