Go back to SDF_ASSERT instead of FATAL_ERROR #1235
Conversation
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
Codecov Report
@@ Coverage Diff @@
## sdf13 #1235 +/- ##
==========================================
+ Coverage 87.19% 87.49% +0.30%
==========================================
Files 126 126
Lines 16301 16239 -62
==========================================
- Hits 14214 14209 -5
+ Misses 2087 2030 -57
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
include/sdf/Element.hh
Outdated
| /// \brief Remove a child element. | ||
| /// \param[in] _child Pointer to the child to remove. | ||
| /// \param[out] _errors Vector of errors. | ||
| public: void RemoveChild(ElementPtr _child, sdf::Errors &_errors); |
There was a problem hiding this comment.
We can't remove this because it breaks ABI :(.
There was a problem hiding this comment.
Oh true! shall we deprecate it instead and remove it on the the next release?
There was a problem hiding this comment.
We can't deprecate it either since our policy is to start a deprecation cycle on a new major release. So, if we want to do that, we'd have to do it in main.
There was a problem hiding this comment.
I'll open a PR to main then. Would an sdf::Error with an ErrorCode::WARNING message suffice?
There was a problem hiding this comment.
Not sure what the ErrorCode::WARNING would be for.
There was a problem hiding this comment.
Oh, we'd use the SDF_DEPRECATED macro for that.
There was a problem hiding this comment.
Yeah, GZ_DEPRECATED. I guess SDF_DEPRECATED is deprecated 😁
We can discuss this on the followup PR, but if there's a chance the sdf::Error might be used by RemoveChild, it might be better to deprecate the overload that doesn't take sdf::Error.
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
|
Brought |
|
Ah, now there's a compiler warning saying |
|
Yup, any suggestion on how to avoid it or shall we just live with it? |
include/sdf/Element.hh
Outdated
| /// \brief Remove a child element. | ||
| /// \param[in] _child Pointer to the child to remove. | ||
| /// \param[out] _errors Vector of errors. | ||
| public: void RemoveChild(ElementPtr _child, sdf::Errors &_errors); |
There was a problem hiding this comment.
Not sure what the ErrorCode::WARNING would be for.
Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> i Signed-off-by: Marco A. Gutierrez <marco@openrobotics.org>
f91e3d0 to
6b9c929
Compare
🦟 Bug fix
Summary
It seems quite hard to guarantee the same flow of the execution of
SDF_ASSERTwhen using aFATAL_ERRORinstead. Because of this, as explained by this comment, this PR reverts all previousSDF_ASSERTchanges intoFATAL_ERRORdone by the following PRs:#1123
#1095
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.🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸