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 #84

Merged
merged 3 commits into from
Dec 8, 2022
Merged

ign -> gz Migrate Ignition headers #84

merged 3 commits into from
Dec 8, 2022

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Dec 1, 2022

See: gazebo-tooling/release-tools#784

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.

@nkoenig nkoenig requested a review from scpeters December 1, 2022 18:40
@nkoenig nkoenig requested a review from azeey as a code owner December 1, 2022 18:40
@nkoenig nkoenig mentioned this pull request Dec 1, 2022
Open
@codecov
Copy link

codecov bot commented Dec 1, 2022 •
edited
Loading

Codecov Report

Merging #84 (91e1cda) into ign-utils1 (a154873) will not change coverage.
The diff coverage is 89.85%.

❗ Current head 91e1cda differs from pull request most recent head 38cdb7f. Consider uploading reports for the commit 38cdb7f to get more accurate results

@@             Coverage Diff             @@
##           ign-utils1      #84   +/-   ##
===========================================
  Coverage       91.97%   91.97%           
===========================================
  Files               6        6           
  Lines             137      137           
===========================================
  Hits              126      126           
  Misses             11       11
Impacted Files Coverage Δ
include/gz/utils/detail/DefaultOps.hh 100.00% <ø> (ø)
include/gz/utils/detail/ImplPtr.hh 91.17% <ø> (ø)
src/Environment.cc 95.23% <ø> (ø)
cli/include/gz/utils/cli/GzFormatter.hpp 88.13% <88.13%> (ø)
include/gz/utils/ImplPtr.hh 100.00% <100.00%> (ø)
include/gz/utils/NeverDestroyed.hh 100.00% <100.00%> (ø)

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

scpeters
scpeters reviewed Dec 1, 2022
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.

do you mind squashing the last 3 commits? It's easier to review if the first commit just renames the headers and then a second commit modifies things

include/ignition/utils/config.hh Show resolved Hide resolved
@nkoenig nkoenig force-pushed the fortress_gz_headers branch from b182e11 to 12f3e04 Compare December 1, 2022 21:19
@nkoenig
Copy link
Contributor Author

nkoenig commented Dec 1, 2022

do you mind squashing the last 3 commits? It's easier to review if the first commit just renames the headers and then a second commit modifies things

Done.

scpeters
scpeters reviewed Dec 1, 2022
View reviewed changes
include/ignition/utils/config.hh Outdated Show resolved Hide resolved
include/gz/utils/NeverDestroyed.hh Outdated Show resolved Hide resolved
@nkoenig nkoenig force-pushed the fortress_gz_headers branch from 9d6f183 to 0630a8e Compare December 1, 2022 22:23
@nkoenig
Copy link
Contributor Author

nkoenig commented Dec 1, 2022

Should I also update the contents of the cli directory?

@scpeters
Copy link
Member

scpeters commented Dec 2, 2022

Should I also update the contents of the cli directory?

yes, good point. I would follow the pattern on the gz-utils2 branch

@nkoenig
Copy link
Contributor Author

nkoenig commented Dec 5, 2022

Should I also update the contents of the cli directory?

yes, good point. I would follow the pattern on the gz-utils2 branch

Great. I update the CLI directory in a few commits that should make it easy to review:

  1. Moved ignition directories to gz: 01326df
  2. Copied the ignition directories from gz-utils2: 9822a1a
  3. Renamed IgnitionFormatter.hpp: b539c89
  4. Updated CMake and gz naming in GzFormatter.hpp: 91e1cda

scpeters
scpeters requested changes Dec 7, 2022
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 have some small changes I'd like to make to whitespace and squashing commits and will push directly to this branch with my fixes later today

Nate Koenig added 2 commits December 7, 2022 21:33
@scpeters
Migrate headers (#84)
5fb4a9b 
Rename IgnitionFormatter.hpp GzFormatter.hpp

Signed-off-by: Nate Koenig <[email protected]>
@scpeters
Add ignition redirection headers (#84)
7864b66 
Copied in the cli/include ignition directories
from the gz-utils2 branch and updated
GzFormatter.hpp.

Signed-off-by: Nate Koenig <[email protected]>
@scpeters scpeters force-pushed the fortress_gz_headers branch from 91e1cda to 32698dc Compare December 8, 2022 18:50
@scpeters scpeters force-pushed the fortress_gz_headers branch from 32698dc to 38cdb7f Compare December 8, 2022 18:52
scpeters
scpeters approved these changes Dec 8, 2022
View reviewed changes
Copy link
Member

@scpeters scpeters left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm made some more changes and squashed putting all file renames in the first commit, most documentation changes in the last commit, and everything else in the middle commit

@nkoenig nkoenig merged commit ec24c32 into ign-utils1 Dec 8, 2022
@nkoenig nkoenig deleted the fortress_gz_headers branch December 8, 2022 21:49
@scpeters scpeters restored the fortress_gz_headers branch December 8, 2022 23:50
@scpeters scpeters deleted the fortress_gz_headers branch December 9, 2022 07:25
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

@mjcarroll mjcarroll mjcarroll approved these changes

@azeey azeey Awaiting requested review from azeey azeey is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nkoenig @scpeters @mjcarroll