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

ign -> gz Namespace Migration : gz-common #356

Merged
merged 29 commits into from
May 28, 2022
Merged

ign -> gz Namespace Migration : gz-common #356

merged 29 commits into from
May 28, 2022

Conversation

methylDragon
Copy link
Contributor

@methylDragon methylDragon requested a review from mjcarroll as a code owner May 18, 2022 05:23
@github-actions github-actions bot added the 🌱 garden Ignition Garden label May 18, 2022
@methylDragon methylDragon mentioned this pull request May 18, 2022
Closed
@methylDragon methylDragon force-pushed the namespace_migration branch from 0c350c9 to 5aabd90 Compare May 18, 2022 05:24
@chapulina chapulina added ign to gz Renaming Ignition to Gazebo. needs upstream release Blocked by a release of an upstream library labels May 18, 2022
@methylDragon methylDragon force-pushed the namespace_migration branch 3 times, most recently from fc57967 to abfe3e8 Compare May 20, 2022 12:44
@scpeters scpeters mentioned this pull request May 22, 2022
Closed
8 tasks
chapulina
chapulina reviewed May 24, 2022
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM besides the comments below.

include/gz/common/Console.hh Outdated Show resolved Hide resolved
Migration.md Show resolved Hide resolved
test/plugins/BadPluginAlign.cc Outdated
@@ -15,20 +15,20 @@
*
*/

#include "ignition/common/PluginMacros.hh"
#include "gz/common/PluginMacros.hh"
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh I'm undoing the plugin header migration on

We can either merge that first and update this PR, or the other way around. I think it may be easier to do the former so we have less changes to update here. I need to address reviewer feedback on that PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do the former! I'll be updating this one in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rebased after the merge of the PR!

The rebase was not nice to do, and I had to do some manual additions and changes, might want to take another look at the changes:

@chapulina chapulina mentioned this pull request May 24, 2022
Merged
9 tasks
@methylDragon methylDragon force-pushed the namespace_migration branch from 50ea0fb to c02cc76 Compare May 25, 2022 22:58
methylDragon added 15 commits May 26, 2022 14:22
@methylDragon
Update header guards
3895483 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate namespaces
b21b14a 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Add migration line
67a61c5 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Add deprecation test
8d128cc 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Implement deprecation trick
f8fdbef 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Include config.hh
031e51b 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Use pragma deprecation message instead
ece343a 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Ticktock config macros
9d1410c 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Mention gz headers in migration guide
ccb204e 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate includes to gz
9065688 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate Export.hh macros and update deprecated test
2d8d6aa 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Greedy macro migration
2590b68 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate github links
06f37f2 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate user-facing Ignition to Gazebo
29318c6 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Tick-tock logging macros
6c815b1 
Signed-off-by: methylDragon <methylDragon@gmail.com>
methylDragon added 10 commits May 26, 2022 14:28
@methylDragon
Migrate gz-common logging calls
5e8c1a1 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Include config.hh in recursive directories and <lib>.hh
2472852 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Gazebo Robotics -> Gazebo
74213b1 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
ignitionrobotics -> gazebosim
a42a6f9 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Revert migration for fuel.ignitionrobotics.org
7009330 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Rollback osrf-migration.github.io migrations
862595e 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Add "Deprecated" to comment, and add note for console deprecations
86a238f 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate igndbg -> gzdbg
78efd1c 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Post-rebase header rectification (header guards and config reordering)
d6211d1 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Suppress ignition pragmas for plugin usage in tests and sources
c6c7825 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the namespace_migration branch from c02cc76 to c6c7825 Compare May 26, 2022 21:49
@methylDragon
Migrate namespace ignition -> namespace gz
0a80e8e 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate ignition:: -> gz::
d934c8b 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Revert namespace change for config.hh
7552295 
Co-authored-by: Louise Poubel <louise@openrobotics.org>

Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the namespace_migration branch from c5db01c to 7552295 Compare May 26, 2022 23:33
@methylDragon
Fix internal non-export DETAIL_IGN_PLUGIN_VISIBLE macro
7f5dd4e 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the namespace_migration branch from 2220535 to 7f5dd4e Compare May 26, 2022 23:58
chapulina
chapulina approved these changes May 27, 2022
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM with 🟢 CI!

@chapulina chapulina merged commit c021582 into main May 28, 2022
@chapulina chapulina deleted the namespace_migration branch May 28, 2022 21:08
@methylDragon methylDragon mentioned this pull request May 29, 2022
Closed
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label May 29, 2022
scpeters
scpeters reviewed Jun 1, 2022
View reviewed changes
include/ignition/common/Plugin.hh
#include <gz/common/Export.hh>
#include <gz/utils/SuppressWarning.hh>

namespace ignition
namespace gz
Copy link
Member

Choose a reason for hiding this comment

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

was it intentional to change the namespace for the deprecated Plugin header files to gz? It would be a stronger signal of deprecation if gz::common::Plugin didn't exist. On the other hand, this way people will get deprecation warnings instead of compilation failures if they search / replace ignition:: for gz:: everywhere. Just checking that it was intentional

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah it's not ideal but there are some advantages to it, see this discussion: #356 (comment)

luca-della-vedova pushed a commit that referenced this pull request Jul 27, 2022
@methylDragon @chapulina @luca-della-vedova
ign -> gz: namespaces and logging functions (#356)
604e03f 
Signed-off-by: methylDragon <methylDragon@gmail.com>

Co-authored-by: Louise Poubel <louise@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina approved these changes

@scpeters scpeters scpeters left review comments

@mjcarroll mjcarroll Awaiting requested review from mjcarroll

Assignees
No one assigned
Labels
🌱 garden Ignition Garden ign to gz Renaming Ignition to Gazebo.
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@methylDragon @scpeters @chapulina