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

Tidy nested namespaces #396

Merged
merged 4 commits into from
Jan 5, 2024
Merged

Tidy nested namespaces #396

merged 4 commits into from
Jan 5, 2024

Conversation

mjcarroll
Copy link
Contributor

@mjcarroll mjcarroll commented Jan 4, 2024 •
edited
Loading

Part of gazebosim/gz-sim#2276

  • Condense nested namespaces and remove "using namespace" where possible: 177c775
  • Split some larger headers into corresponding pieces: a02c156
  • Split out some of the types that were defined in larger headers: 097ce9e

@j-rivero I removed some TODOs about renaming the ResultType::DELETE and HttpMethod::DELETE entries. I think that keeping the DELETE name is the most appropriate with the undefs. Since we are now using it purely in enum class and the other use is part of the standard access rights I think that the chance of collision is low enough to not worry about.

@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Jan 4, 2024
@mjcarroll mjcarroll mentioned this pull request Jan 4, 2024
Open
16 tasks
@mjcarroll mjcarroll force-pushed the mjcarroll/tidy_namespaces branch from 7d3dcad to 79daaa5 Compare January 4, 2024 21:03
@mjcarroll
Tidy nested namespaces
177c775 
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll mjcarroll force-pushed the mjcarroll/tidy_namespaces branch from 79daaa5 to 177c775 Compare January 4, 2024 21:06
@mjcarroll
Split Client and Server Config files
a02c156 
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll
Split out more types
097ce9e 
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll mjcarroll marked this pull request as ready for review January 4, 2024 21:40
@mjcarroll mjcarroll requested a review from nkoenig as a code owner January 4, 2024 21:40
@mjcarroll mjcarroll requested a review from ahcorde January 5, 2024 15:33
ahcorde
ahcorde requested changes Jan 5, 2024
View reviewed changes
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Some minor comments

include/gz/fuel_tools/HttpMethod.hh Outdated Show resolved Hide resolved
include/gz/fuel_tools/ResultType.hh Outdated Show resolved Hide resolved
include/gz/fuel_tools/ResultType.hh Outdated Show resolved Hide resolved
include/gz/fuel_tools/ServerConfig.hh Outdated Show resolved Hide resolved
include/gz/fuel_tools/ServerConfig.hh Outdated Show resolved Hide resolved
src/ModelIterPrivate.hh Show resolved Hide resolved
src/ServerConfig.cc Outdated Show resolved Hide resolved
src/ServerConfig_TEST.cc Outdated Show resolved Hide resolved
@mjcarroll
Address reviewer feedback
8a56903 
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll mjcarroll merged commit 93ec03d into gz-fuel-tools9 Jan 5, 2024
9 checks passed
@mjcarroll mjcarroll deleted the mjcarroll/tidy_namespaces branch January 5, 2024 18:36
caguero pushed a commit that referenced this pull request Feb 2, 2024
@mjcarroll @caguero
Tidy nested namespaces (#396)
1019108 
Signed-off-by: Michael Carroll <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahcorde ahcorde ahcorde approved these changes

@azeey azeey azeey approved these changes

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

Assignees
No one assigned
Labels
🎵 harmonic Gazebo Harmonic
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mjcarroll @azeey @ahcorde