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

Parse correctly OGRE2_RESOURCE_PATH on Windows #996

Merged
merged 1 commit into from
May 28, 2024

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented May 2, 2024

🦟 Bug fix

Fixes wrong handling of OGRE2_RESOURCE_PATH on Windows.

Summary

It was assume that the OGRE2_RESOURCE_PATH was a :-delimiter list of paths. However, on Windows : is the separator between the drive letter and the path, so ; should be used instead.

Everything was working fine as C:\this\is\a\path and \this\is\a\path are both valid paths if the active drive is C:\, but as soon as the current active directory was for example on D:, OGRE2_RESOURCE_PATH was not read correctly, see conda-forge/gz-sim-feedstock#57 .

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

@traversaro traversaro requested a review from iche033 as a code owner May 2, 2024 15:27
@github-actions github-actions bot added the 🌱 garden Ignition Garden label May 2, 2024
@traversaro traversaro force-pushed the fixwinwrongpartition branch 4 times, most recently from 799ebd5 to d9edf09 Compare May 2, 2024 17:49
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!

@iche033
Copy link
Contributor

iche033 commented May 2, 2024

looks good to me. Can you fix DCO?

@traversaro traversaro force-pushed the fixwinwrongpartition branch from 118fdca to 07c28d3 Compare May 2, 2024 19:04
@traversaro
Copy link
Contributor Author

looks good to me. Can you fix DCO?

Done!

Copy link

codecov bot commented May 2, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 77.68%. Comparing base (f95826f) to head (07c28d3).

Files Patch % Lines
ogre2/src/Ogre2RenderEngine.cc 50.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           gz-rendering7     #996   +/-   ##
==============================================
  Coverage          77.68%   77.68%           
==============================================
  Files                170      170           
  Lines              14733    14733           
==============================================
  Hits               11445    11445           
  Misses              3288     3288           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iche033 iche033 merged commit 65b5ed8 into gazebosim:gz-rendering7 May 28, 2024
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants