-
Notifications
You must be signed in to change notification settings - Fork 98
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
Error when delimiter "::" found in element name in SDFormat 1.8 #515
Conversation
Signed-off-by: Jenn Nguyen <[email protected]>
Signed-off-by: Jenn Nguyen <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #515 +/- ##
=======================================
Coverage 87.81% 87.81%
=======================================
Files 66 66
Lines 9321 9347 +26
=======================================
+ Hits 8185 8208 +23
- Misses 1136 1139 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good; I just left some minor comments
nice work!
src/parser_TEST.cc
Outdated
<< " </model>" | ||
<< "</sdf>"; | ||
|
||
EXPECT_FALSE(sdf::readString(stream.str(), sdf)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you could pass an sdf::Errors
object to sdf::readString
and check its contents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great!
Signed-off-by: Jenn Nguyen <[email protected]>
Waiting for #504 to be merged before merging this one |
Closes #420
Summary
Error when the
::
delimiter is found inname
attributes for files that are SDFormat >= 1.8Checklist
sh tools/code_check.sh
)another open pull request
to support the maintainers
Note to maintainers: Remember to use Squash-Merge