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 CMake, Python, Partial Source, and File Migrations : gz-launch #176

Merged
merged 2 commits into from
Jul 15, 2022
Merged

ign -> gz CMake, Python, Partial Source, and File Migrations : gz-launch #176

merged 2 commits into from
Jul 15, 2022

Conversation

methylDragon
Copy link
Contributor

@methylDragon methylDragon requested a review from nkoenig as a code owner July 12, 2022 18:15
@methylDragon methylDragon mentioned this pull request Jul 12, 2022
Closed
32 tasks
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jul 12, 2022
@methylDragon
Copy link
Contributor Author

@osrf-jenkins run tests please!

1 similar comment
@methylDragon
Copy link
Contributor Author

@osrf-jenkins run tests please!

@methylDragon methylDragon force-pushed the ci_matching_branch/file_and_source_migration branch from bebbd6c to ebdd907 Compare July 12, 2022 20:57
@methylDragon
Copy link
Contributor Author

@osrf-jenkins run tests please!

1 similar comment
@methylDragon
Copy link
Contributor Author

@osrf-jenkins run tests please!

@chapulina chapulina added the ign to gz Renaming Ignition to Gazebo. label Jul 13, 2022
@methylDragon methylDragon force-pushed the ci_matching_branch/file_and_source_migration branch from ebdd907 to b4dcde2 Compare July 13, 2022 00:16
chapulina
chapulina approved these changes Jul 13, 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 green CI. We should wait for gazebosim/gz-sim#1589 to be merged first.

Migration.md Outdated Show resolved Hide resolved
plugins/websocket_server/WebsocketServer.cc Outdated
@@ -806,7 +806,7 @@ void WebsocketServer::OnMessage(int _socketId, const std::string &_msg)
bool result;
unsigned int timeout = 2000;

bool executed = this->node.Request("/gazebo/worlds",
bool executed = this->node.Request("/sim/worlds",
Copy link
Contributor

Choose a reason for hiding this comment

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

This requires gazebosim/gz-sim#1589

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually we decided to revert these changes

@codecov
Copy link

codecov bot commented Jul 13, 2022 •
edited
Loading

Codecov Report

Merging #176 (db67006) into main (e05d212) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #176   +/-   ##
=======================================
  Coverage   32.49%   32.49%           
=======================================
  Files           4        4           
  Lines         834      834           
=======================================
  Hits          271      271           
  Misses        563      563
Impacted Files Coverage Δ
src/cmd/launch_main.cc 69.11% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e05d212...db67006. Read the comment docs.

@methylDragon methylDragon force-pushed the ci_matching_branch/file_and_source_migration branch from b4dcde2 to 357e903 Compare July 13, 2022 08:52
methylDragon added a commit that referenced this pull request Jul 13, 2022
@methylDragon
Partial source and cmake migrations (#176)
94ca6f1 
Signed-off-by: methylDragon <[email protected]>
@methylDragon methylDragon force-pushed the ci_matching_branch/file_and_source_migration branch from 357e903 to 94ca6f1 Compare July 13, 2022 08:53
@methylDragon methylDragon added the needs upstream release Blocked by a release of an upstream library label Jul 13, 2022
methylDragon added a commit that referenced this pull request Jul 13, 2022
@methylDragon
Partial source and cmake migrations (#176)
cf8e781 
Signed-off-by: methylDragon <[email protected]>
@methylDragon methylDragon force-pushed the ci_matching_branch/file_and_source_migration branch from 94ca6f1 to cf8e781 Compare July 13, 2022 08:57
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Jul 14, 2022
chapulina
chapulina suggested changes Jul 14, 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.

Just need to revert the topic change

@methylDragon
Copy link
Contributor Author

Just need to revert the topic change

Just need to revert the topic change

Forgot to comment that it was reverted!

https://github.com/gazebosim/gz-launch/blob/ci_matching_branch/file_and_source_migration/plugins/websocket_server/WebsocketServer.cc

chapulina
chapulina reviewed Jul 14, 2022
View reviewed changes
plugins/websocket_server/WebsocketServer.cc Outdated
@@ -806,7 +806,7 @@ void WebsocketServer::OnMessage(int _socketId, const std::string &_msg)
bool result;
unsigned int timeout = 2000;

bool executed = this->node.Request("/gazebo/worlds",
bool executed = this->node.Request("/sim/worlds",
Copy link
Contributor

Choose a reason for hiding this comment

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

I still see it here

gz-launch/plugins/websocket_server/WebsocketServer.cc

Lines 809 to 810 in cf8e781

bool executed = this->node.Request("/sim/worlds",
req, timeout, rep, result);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops: db67006

@methylDragon methylDragon force-pushed the ci_matching_branch/file_and_source_migration branch from cf8e781 to db67006 Compare July 15, 2022 00:40
@methylDragon methylDragon mentioned this pull request Jul 15, 2022
Closed
@methylDragon
Copy link
Contributor Author

I also fixed the missing pre1 here, in addition to reverting sim/worlds, see comparison: https://github.com/gazebosim/gz-launch/compare/cf8e781b380e1109c382cc2026dcfd29acfc27ec..db670067ca0cb706c1cc18383d82c220e05011cc

@methylDragon methylDragon enabled auto-merge (rebase) July 15, 2022 01:46
@methylDragon methylDragon merged commit 7efd275 into main Jul 15, 2022
@methylDragon methylDragon deleted the ci_matching_branch/file_and_source_migration branch July 15, 2022 05:37
methylDragon added a commit that referenced this pull request Jul 15, 2022
@methylDragon
Migrate launchfiles and js (#176)
6b4ca47 
Signed-off-by: methylDragon <[email protected]>
methylDragon added a commit that referenced this pull request Jul 20, 2022
@methylDragon
Partial source and cmake migrations (#176)
4f29674 
Signed-off-by: methylDragon <[email protected]>
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

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

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.
2 participants
@methylDragon @chapulina