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.

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

Fix generator expression #374

Merged
merged 5 commits into from
Aug 29, 2023
Merged

Conversation

mjcarroll
Copy link
Contributor

Fixes the issue found here where there isn't an exe extension: gazebosim/gz-sim#2087 (comment)

Signed-off-by: Michael Carroll <[email protected]>
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Aug 29, 2023
@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Merging #374 (079ce6d) into gz-msgs10 (f41e1e2) will increase coverage by 0.03%.
The diff coverage is 95.00%.

❗ Current head 079ce6d differs from pull request most recent head e75b567. Consider uploading reports for the commit e75b567 to get more accurate results

@@              Coverage Diff              @@
##           gz-msgs10     #374      +/-   ##
=============================================
+ Coverage      97.21%   97.25%   +0.03%     
=============================================
  Files             27       27              
  Lines           1150     1165      +15     
=============================================
+ Hits            1118     1133      +15     
  Misses            32       32              
Files Changed Coverage Δ
core/src/DynamicFactory.cc 86.36% <66.66%> (-0.74%) ⬇️
core/src/MessageFactory.cc 97.87% <100.00%> (+3.42%) ⬆️

Signed-off-by: Michael Carroll <[email protected]>
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

LGTM! I'll open an issue for the GZ_MSGS_GEN_EXECUTABLE regression. Also, there's one debug print that needs to be removed.

@@ -79,6 +79,8 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths)
if (_paths.empty())
return;

std::cout << "DynamicFactory::LoadDescriptors(" << _paths << "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove

@azeey azeey added the beta Targeting beta release of upcoming collection label Aug 29, 2023
@azeey
Copy link
Contributor

azeey commented Aug 29, 2023

Oh, and this should target gz-msgs10 now.

Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll mjcarroll changed the base branch from main to gz-msgs10 August 29, 2023 20:04
@azeey
Copy link
Contributor

azeey commented Aug 29, 2023

This fixes one of the two test failures in gazebosim/gz-transport#434. UNIT_ParamCommandAPI_TEST still fails because of the deprecation message, but I think we should fix that there.

Signed-off-by: Michael Carroll <[email protected]>
@azeey
Copy link
Contributor

azeey commented Aug 29, 2023

I'm going to make the ABI checker not required for now. I think it's failing because it's building on Focal.

@mjcarroll mjcarroll merged commit 0a47973 into gz-msgs10 Aug 29, 2023
4 of 5 checks passed
@mjcarroll mjcarroll deleted the mjcarroll/fix_generator_expression branch August 29, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants