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 Migrate Ignition Headers : gz-gui #466

Merged
merged 8 commits into from
Nov 30, 2022
Merged

ign -> gz Migrate Ignition Headers : gz-gui #466

merged 8 commits into from
Nov 30, 2022

Conversation

methylDragon
Copy link
Contributor

@methylDragon methylDragon requested a review from jennuine as a code owner August 12, 2022 22:37
@methylDragon methylDragon mentioned this pull request Aug 12, 2022
Open
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Aug 12, 2022
@chapulina chapulina added the ign to gz Renaming Ignition to Gazebo label Aug 15, 2022
@codecov
Copy link

codecov bot commented Aug 18, 2022 •
edited
Loading

Codecov Report

Merging #466 (ccaee8e) into ign-gui3 (721eca0) will not change coverage.
The diff coverage is 90.90%.

❗ Current head ccaee8e differs from pull request most recent head 90f1694. Consider uploading reports for the commit 90f1694 to get more accurate results

@@            Coverage Diff            @@
##           ign-gui3     #466   +/-   ##
=========================================
  Coverage     73.82%   73.82%           
=========================================
  Files            30       30           
  Lines          3270     3270           
=========================================
  Hits           2414     2414           
  Misses          856      856
Impacted Files Coverage Δ
src/Dialog.cc 100.00% <ø> (ø)
src/DragDropModel.cc 100.00% <ø> (ø)
src/MainWindow.cc 95.80% <ø> (ø)
src/Plugin.cc 82.68% <ø> (ø)
src/SearchModel.cc 96.55% <ø> (ø)
src/plugins/grid_3d/Grid3D.cc 10.58% <ø> (ø)
src/plugins/grid_3d/Grid3D.hh 100.00% <ø> (ø)
src/plugins/image_display/ImageDisplay.cc 99.31% <ø> (ø)
src/plugins/image_display/ImageDisplay.hh 100.00% <ø> (ø)
src/plugins/publisher/Publisher.cc 90.78% <ø> (ø)
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@methylDragon methylDragon force-pushed the citadel_header_redirects branch 9 times, most recently from bdd04c8 to f963ab5 Compare August 19, 2022 03:47
@jennuine jennuine mentioned this pull request Aug 19, 2022
Open
9 tasks
@methylDragon methylDragon changed the title ign -> gz Reverse Ignition Headers : gz-gui ign -> gz Migrate Ignition Headers : gz-gui Aug 22, 2022
@methylDragon
Migrate headers
c0c52f0 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Add redirection headers
63271c6 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Migrate include statements
b804f63 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the citadel_header_redirects branch from f963ab5 to b804f63 Compare August 22, 2022 22:18
jennuine
jennuine reviewed Aug 23, 2022
View reviewed changes
Copy link
Contributor

@jennuine jennuine left a comment

Choose a reason for hiding this comment

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

Some of these suggestions may be ignored depending on your plan/workflow for renaming but the code comments in the migrated headers should be updated from ign* to gz/gazebo

include/gz/gui/Application.hh Outdated Show resolved Hide resolved
include/gz/gui/Application.hh Show resolved Hide resolved
include/gz/gui/Application.hh Outdated Show resolved Hide resolved
include/gz/gui/Application.hh Show resolved Hide resolved
include/gz/gui/CMakeLists.txt Show resolved Hide resolved
test/integration/ExamplesBuild_TEST.cc Outdated Show resolved Hide resolved
test/integration/Examples_TEST.cc Outdated Show resolved Hide resolved
test/integration/deprecated_TEST.cc Show resolved Hide resolved
tutorials/03_plugins.md Outdated Show resolved Hide resolved
tutorials/03_plugins.md Outdated Show resolved Hide resolved
@jennuine jennuine added the needs upstream release Blocked by a release of an upstream library label Aug 23, 2022
@methylDragon methylDragon force-pushed the citadel_header_redirects branch from 86308cf to d7e8f75 Compare August 26, 2022 21:01
@methylDragon
Copy link
Contributor Author

Some of these suggestions may be ignored depending on your plan/workflow for renaming but the code comments in the migrated headers should be updated from ign* to gz/gazebo

Thanks for the reviews!

I think the docs/comments migration will be happening in a separate (easier to merge) wave, since they can really make reviewing some of the larger libraries difficult (e.g. sim), same for the github URL changes.

For the macro and CMake macro changes, I don't think the upstream changes that support them has been made, since only headers were migrated this wave.

I'll fold in the broken URL changes though, thanks for catching those! 🙇

@methylDragon methylDragon force-pushed the citadel_header_redirects branch 2 times, most recently from 180f036 to 89ae4fa Compare August 30, 2022 20:58
@methylDragon
Leave ignition as primary in headers to fix ABI
516077d 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the citadel_header_redirects branch from 89ae4fa to 516077d Compare September 5, 2022 23:43
@jennuine
Copy link
Contributor

@methylDragon what's the status of this?

@methylDragon
Copy link
Contributor Author

@methylDragon what's the status of this?

This was supposed to be ready but pending the release of upstream libraries. I'm not sure about now though, changes might have occurred between then and now might mean new stuff added might need to be migrated.

@jennuine
Copy link
Contributor

Are there any plans to revisit this?

@methylDragon
Copy link
Contributor Author

Are there any plans to revisit this?

Hmm, I think I won't be able to spend active time on this.. Should I close this PR?

@nkoenig
Copy link
Contributor

nkoenig commented Nov 28, 2022

I'll pick up this PR.

Nate Koenig added 3 commits November 29, 2022 13:45
merged with ign-gui3
e42d685 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
Changed more Ignition names
efafa33 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
Update ignitionrobotics to gazebosim
01b006c 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
jennuine
jennuine approved these changes Nov 30, 2022
View reviewed changes
Changelog.md Outdated Show resolved Hide resolved
Changelog.md Outdated Show resolved Hide resolved
api.md.in Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Changelog.md Outdated Show resolved Hide resolved
include/gz/gui/Helpers.hh Outdated Show resolved Hide resolved
tutorials/01_install.md Show resolved Hide resolved
tutorials/01_install.md Outdated Show resolved Hide resolved
tutorials/01_install.md Outdated Show resolved Hide resolved
tutorials/05_style.md Outdated Show resolved Hide resolved
Update renaming
90f1694 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
@nkoenig nkoenig merged commit b016183 into ign-gui3 Nov 30, 2022
@nkoenig nkoenig deleted the citadel_header_redirects branch November 30, 2022 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jennuine jennuine jennuine approved these changes

@nkoenig nkoenig nkoenig approved these changes

Assignees
No one assigned
Labels
🏰 citadel Ignition Citadel ign to gz Renaming Ignition to Gazebo needs upstream release Blocked by a release of an upstream library
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@methylDragon @jennuine @nkoenig @chapulina