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

Emit an error instead of a warning when a file has multiple root level model, actor or light elements #619

Merged
merged 1 commit into from
Jul 14, 2021

Conversation

azeey
Copy link
Collaborator

@azeey azeey commented Jul 9, 2021

Summary

#433 and #444 made it so that only one root level model, actor or light can be contained in an SDFormat file. So as to not break existing behavior, only a warning was issued if more than one is found. These have now been converted to errors. This PR also removes deprecated functions for accessing multiple model, actor or light elements from sdf::Root.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

model, actor or light elements.

Also, remove deprecated functions for accessing multiple model, actor or
light elements from the Root DOM.

Signed-off-by: Addisu Z. Taddese <[email protected]>
@azeey azeey requested a review from marcoag July 9, 2021 16:40
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Jul 9, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #619 (07e6fa1) into main (d8f8415) will decrease coverage by 0.02%.
The diff coverage is 85.18%.

❗ Current head 07e6fa1 differs from pull request most recent head 104944f. Consider uploading reports for the commit 104944f to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #619      +/-   ##
==========================================
- Coverage   87.62%   87.59%   -0.03%     
==========================================
  Files          72       72              
  Lines       10534    10513      -21     
==========================================
- Hits         9230     9209      -21     
  Misses       1304     1304              
Impacted Files Coverage Δ
src/Root.cc 93.15% <83.67%> (+1.53%) ⬆️
src/parser.cc 84.31% <100.00%> (ø)
src/Utils.cc 82.81% <0.00%> (-3.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8f8415...104944f. Read the comment docs.

@azeey azeey self-assigned this Jul 9, 2021
@marcoag
Copy link
Member

marcoag commented Jul 12, 2021

LGTM

@azeey azeey merged commit 2d298f0 into gazebosim:main Jul 14, 2021
@azeey azeey deleted the multiple_root_element_error branch July 14, 2021 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants