Port embedSdf script from Ruby to Python3 and provide unittests #884
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@osrf-jenkins run tests please |
scpeters
reviewed
Mar 17, 2022
Bi0T1N
force-pushed
the
embeddedsdf_refactoring
branch
from
2fb48f1
to
df97b05
Compare
scpeters
reviewed
Mar 18, 2022
Bi0T1N
force-pushed
the
embeddedsdf_refactoring
branch
2 times, most recently
from
625bc4d
to
934cac5
Compare
chapulina
added
the
needs upstream release
Bi0T1N
force-pushed
the
embeddedsdf_refactoring
branch
from
55a990a
to
0862e66
Compare
Codecov Report
@@ Coverage Diff @@
## main #884 +/- ##
==========================================
+ Coverage 87.20% 87.21% +0.01%
==========================================
Files 124 124
Lines 16229 16229
==========================================
+ Hits 14152 14154 +2
+ Misses 2077 2075 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
azeey
reviewed
Aug 5, 2022
Bi0T1N
force-pushed
the
embeddedsdf_refactoring
branch
from
939219a
to
7395479
Compare
Bi0T1N
requested review from
azeey
and removed request for
mjcarroll and
scpeters
Bi0T1N
force-pushed
the
embeddedsdf_refactoring
branch
from
7395479
to
e8f56ba
Compare
|
I fixed the merge conflict that was due to the |
chapulina
removed
the
needs upstream release
this uses internal functions from GoogleTests to capture the stderr message but according to the documentation it should be fine to use them: // All macros ending with _ and symbols defined in an // internal namespace are subject to change without notice. Code // outside Google Test MUST NOT USE THEM DIRECTLY. Macros that don't // end with _ are part of Google Test's public API and can be used by // code outside Google Test. see https://github.com/google/googletest/blob/main/googletest/include/gtest/internal/gtest-port.h Signed-off-by: Bi0T1N <[email protected]>
- removes Ruby dependency (see gazebosim#274) - improvements for cpplint checks (e.g. copyright header) Signed-off-by: Bi0T1N <[email protected]>
it should not be operating system dependent because it is hardcoded to '/' in the C++ file that performs the lookup Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Bi0T1N <[email protected]>
This reverts commit 2322458. Signed-off-by: Bi0T1N <[email protected]>
it's the same approach as in parser_TEST.cc Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Bi0T1N <[email protected]>
Signed-off-by: Bi0T1N <[email protected]>
Bi0T1N
force-pushed
the
embeddedsdf_refactoring
branch
from
e16e985
to
bbe6714
Compare
|
Apparently I misinterpreted the comments and made the changes to the wrong project. Anyway, rebased my changes on main and added Python interpreter as a build dependency with |
mjcarroll
approved these changes
Feb 2, 2023
|
@scpeters any remaining issues? |
scpeters
approved these changes
Feb 6, 2023
mjcarroll
added a commit
that referenced
this pull request
Feb 16, 2023
Converts the script that creates the .cc file for mapping of SDF filename and SDF content from Ruby to Python. Additionally a few unittests are added for the function that uses the mapping and which wasn't tested before. Signed-off-by: Bi0T1N <[email protected]> Co-authored-by: Bi0T1N <[email protected]> Co-authored-by: Michael Carroll <[email protected]>
mjcarroll
added a commit
that referenced
this pull request
Feb 16, 2023
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]>
7 tasks
mjcarroll
added a commit
that referenced
this pull request
Dec 13, 2023
* Cherry-pick the python3 embedSdf script and tests (#884) 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]> * Improvements to embedSdf script Signed-off-by: Michael Carroll <[email protected]> * Update sdf/CMakeLists.txt Signed-off-by: Michael Carroll <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]> * Update bazel files Signed-off-by: Michael Carroll <[email protected]> * Lint Signed-off-by: Michael Carroll <[email protected]> * Add utils back to parser Signed-off-by: Michael Carroll <[email protected]> * Fix gz_TEST Signed-off-by: Michael Carroll <[email protected]> * Ignore pycache folders Signed-off-by: Michael Carroll <[email protected]> * Fix parser_TEST Signed-off-by: Michael Carroll <[email protected]> * Bazel nits Signed-off-by: Michael Carroll <[email protected]> * Fix path logic Signed-off-by: Michael Carroll <[email protected]> * Fix visibility Signed-off-by: Michael Carroll <[email protected]> * Fix strings Signed-off-by: Michael Carroll <[email protected]> * Use standard vars Signed-off-by: Michael Carroll <[email protected]> * Fix string conversions Signed-off-by: Michael Carroll <[email protected]> * Fix string conversions Signed-off-by: Michael Carroll <[email protected]> * Fix embedded sdf Signed-off-by: Michael Carroll <[email protected]> * Fix converter test Signed-off-by: Michael Carroll <[email protected]> * Lint Signed-off-by: Michael Carroll <[email protected]> * Update bazel files Signed-off-by: Michael Carroll <[email protected]> * Add detail prefix Signed-off-by: Michael Carroll <[email protected]> * Fix test path Signed-off-by: Michael Carroll <[email protected]> * Set homepath on Windows Signed-off-by: Michael Carroll <[email protected]> --------- Signed-off-by: Michael Carroll <[email protected]> Signed-off-by: Michael Carroll <[email protected]> Co-authored-by: Bi0T1N <[email protected]> Co-authored-by: Bi0T1N <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎉 New feature
Removes the Ruby dependency and thus is related to/partially solves #274 (for complete Ruby dependency removal #643 should be merged as well).
Summary
This PR converts the script that creates the
.ccfile for mapping of SDF filename and SDF content from Ruby to Python. Additionally a few unittests are added for the function that uses the mapping and which wasn't tested before.For details see also the commit messages.
Note: Since I wasn't sure if this change counts as an API/ABI change or not, I targeted it to main.
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.