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

parser: Use encapsulated string constants for non-file sources #551

Merged
merged 1 commit into from
May 10, 2021

Conversation

EricCousineau-TRI
Copy link
Collaborator

@EricCousineau-TRI EricCousineau-TRI commented Apr 29, 2021

🦟 Bug fix

Fixes #536, follow-up to #537
Downstream: gazebosim/gz-sim#794

Summary

Takes prior revision of #537.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed) - N/A
  • Updated migration guide (as needed) - N/A
  • Code check passed (In source directory, run sh tools/code_check.sh)
  • 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

@azeey
Copy link
Collaborator

azeey commented Apr 29, 2021

I think this will change behavior in a stable release. The string "data-string" is used in a stable release of ign-gazebo. Can we target this to main?

@chapulina
Copy link
Contributor

I think this will change behavior in a stable release.

True. How about keeping the string as data-string on this branch and updating it to <data-string> on main?

Signed-off-by: Eric Cousineau <eric.cousineau@tri.global>
@EricCousineau-TRI EricCousineau-TRI changed the base branch from sdf11 to main May 4, 2021 22:13
@EricCousineau-TRI
Copy link
Collaborator Author

Done! Retargeted main

Meta: I'm a wee bit confused on release process and expected stability for version branches and all that. Y'all got a page that explains that part?

Copy link
Collaborator

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for quick fixes!

Meta: I'm a wee bit confused on release process and expected stability for version branches and all that. Y'all got a page that explains that part?

I would be curious to learn more about this too

@EricCousineau-TRI
Copy link
Collaborator Author

EricCousineau-TRI commented May 6, 2021

Per VC, ABI checker failure may be due to statefulness; @azeey will try to re-run; simplest mitigation may be to ignore CI results and merge pending approval.
https://build.osrfoundation.org/job/sdformat-abichecker-any_to_any-ubuntu_auto-amd64/1162/console

Most likely, job got queued upon initial PR, but didn't go away when branch was retargeted.

@azeey azeey merged commit a187ef4 into gazebosim:main May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

String parsing results in FilePath as data-string when error occurs
4 participants