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

Backport the python3 embedSdf script variant #1240

Merged
merged 3 commits into from
Feb 17, 2023

Conversation

mjcarroll
Copy link
Contributor

This backports the embedSdf.py script from #884 and adds some improvements to make it usable in support of the sdformat bazel build.

This does not make sdf13 use the embedSdf script by default, instead still preferring the ruby version.

Bi0T1N and others added 2 commits February 16, 2023 11:09
These won't be used as part of the cmake build on this branch, but are
useful for generating the same file from bazel.

Co-authored-by: Bi0T1N <[email protected]>
Co-authored-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Feb 16, 2023
Copy link
Collaborator

@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.

I'm hoping CI is only failing because of the .py->.rb typo.

sdf/CMakeLists.txt Outdated Show resolved Hide resolved
Signed-off-by: Michael Carroll <[email protected]>

Co-authored-by: Addisu Z. Taddese <[email protected]>
@codecov
Copy link

codecov bot commented Feb 16, 2023

Codecov Report

Merging #1240 (4c13292) into sdf13 (f1d13b4) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 4c13292 differs from pull request most recent head a82e65f. Consider uploading reports for the commit a82e65f to get more accurate results

@@            Coverage Diff             @@
##            sdf13    #1240      +/-   ##
==========================================
+ Coverage   87.49%   87.51%   +0.01%     
==========================================
  Files         126      126              
  Lines       16239    16239              
==========================================
+ Hits        14209    14211       +2     
+ Misses       2030     2028       -2     
Impacted Files Coverage Δ
src/SDF.cc 95.23% <0.00%> (+1.19%) ⬆️

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

@mjcarroll
Copy link
Contributor Author

@osrf-jenkins retest this please

@mjcarroll mjcarroll merged commit baa1e9f into sdf13 Feb 17, 2023
@mjcarroll mjcarroll deleted the mjcarroll/backport_embedsdf branch February 17, 2023 19:05
sdf/embedSdf.py Outdated
files = []
for path in arg_pathlist:
# dir separator is hardcoded to '/' in C++ mapping
posix_path = PurePosixPath(path)
Copy link
Member

Choose a reason for hiding this comment

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

we've seen regressions on windows when porting this forward in #1297 (review)

I think we need to keep the PurePosixPath call

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