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

Make ThrowOrPrintError a free internal function #1221

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

azeey
Copy link
Collaborator

@azeey azeey commented Jan 18, 2023

🦟 Bug fix

Summary

#1220 added sdf::Error::ThrowOrPrintError, but after further thought, I think it would be better if we made it a free function inside the internal namespace as this allows us to remove the function without a deprecation cycle. Since the future goal of libsdformat is to remove exceptions, it is likely that we'd want to remove ThrowOrPrintError in the not too distance future.

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.

This allows us to remove the function without a deprecation cycle.

Signed-off-by: Addisu Z. Taddese <[email protected]>
@azeey azeey requested a review from marcoag January 18, 2023 02:32
@azeey azeey requested a review from scpeters as a code owner January 18, 2023 02:32
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jan 18, 2023
@codecov
Copy link

codecov bot commented Jan 18, 2023

Codecov Report

Merging #1221 (1ae23b3) into sdf13 (de8b30d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 1ae23b3 differs from pull request most recent head 34e196b. Consider uploading reports for the commit 34e196b to get more accurate results

@@           Coverage Diff           @@
##            sdf13    #1221   +/-   ##
=======================================
  Coverage   87.20%   87.21%           
=======================================
  Files         124      124           
  Lines       16232    16232           
=======================================
+ Hits        14155    14156    +1     
+ Misses       2077     2076    -1     
Impacted Files Coverage Δ
src/Exception.cc 97.22% <ø> (ø)
src/Error.cc 100.00% <100.00%> (+1.51%) ⬆️
src/Utils.cc 87.64% <100.00%> (ø)

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

@marcoag
Copy link
Member

marcoag commented Jan 18, 2023

Sure makes sense, thanks for the fix!

Signed-off-by: Addisu Z. Taddese <[email protected]>
@azeey
Copy link
Collaborator Author

azeey commented Jan 19, 2023

There's an ABI change, but it's okay since we haven't made a release with #1220.

@azeey azeey merged commit 8ae5929 into gazebosim:sdf13 Jan 19, 2023
@azeey azeey deleted the throw_or_print_free_func branch January 19, 2023 15:59
mjcarroll pushed a commit that referenced this pull request Feb 9, 2023
#1220 added sdf::Error::ThrowOrPrintError, but after further thought, I think it would be better if we made it a free function inside the internal namespace as this allows us to remove the function without a deprecation cycle. Since the future goal of libsdformat is to remove exceptions, it is likely that we'd want to remove ThrowOrPrintError in the not too distance future.

Signed-off-by: Addisu Z. Taddese <[email protected]>
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.

2 participants