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

yaml: Add read_archive Options struct (currently a no-op) #13537

Merged

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Jun 10, 2020

This paves the way for dealing with parser options in a future commit.

Thread the Options through the Serialize/Visit flow but don't use it for anything yet.

Rename ReportMissingYaml to ReportError to avoid being overly specific; this was already used for error messages beyond just missing stuff.

While we're here, also import the unit test with helpful comments and extracting some literal strings into named variables.

This builds on #13533 and is a prerequisite for #13532 towards #13251.


This change is Reviewable

This paves the way for dealing with parser options in a future commit.

Thread the Options through the Serialize/Visit flow but don't use it for
anything yet.

Rename ReportMissingYaml to ReportError to avoid being overly specific;
this was already used for error messages beyond just missing stuff.

While we're here, also import the unit test with helpful comments and
extracting some literal strings into named variables.
@jwnimmer-tri jwnimmer-tri marked this pull request as ready for review June 10, 2020 23:57
@jwnimmer-tri
Copy link
Collaborator Author

+@ggould-tri for feature review, please.

@jamiesnape
Copy link
Contributor

@drake-jenkins-bot linux-focal-gcc-bazel-experimental-release please

Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r1.
Reviewable status: needs at least two assigned reviewers

@jwnimmer-tri
Copy link
Collaborator Author

+@sammy-tri for platform review per schedule, please.

Copy link
Contributor

@sammy-tri sammy-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees ggould-tri(platform),sammy-tri(platform) (waiting on @sammy-tri)

@sammy-tri sammy-tri merged commit 4ea3c4b into RobotLocomotion:master Jun 11, 2020
@jwnimmer-tri jwnimmer-tri deleted the yaml-read-configurable-errors2 branch June 11, 2020 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants