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

Json parse prep #1580

Closed
wants to merge 2 commits into from
Closed

Json parse prep #1580

wants to merge 2 commits into from

Conversation

cgwalters
Copy link
Member

No description provided.

Prep for further changes.
Prep for moving more of our parsing into Rust.  The main
thing here is that for JSON, we need to continue to ignore
unknown fields.  It took me a little while but I eventually
figured out that using `#[serde(flatten)]` works for this.
Seriously: serde is freaking amazingly awesome.
))
fn treefile_parse_stream(
fmt: InputFormat,
input: &mut io::Read,
Copy link
Member

Choose a reason for hiding this comment

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

Minor: why the switch away from a generic here? Seems fine either way, just want to understand the motivation.

Copy link
Member Author

Choose a reason for hiding this comment

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

There were like 2-3 rewrites/false starts here before this patch got split out as a prep for the bigger PR. I don't think there was any deliberate reason, and it does make sense to me to avoid vfuncs here.

Copy link
Member Author

Choose a reason for hiding this comment

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

(will do a followup)

@jlebon
Copy link
Member

jlebon commented Sep 26, 2018

@rh-atomic-bot r+ d79b1f9

@rh-atomic-bot
Copy link

⚡ Test exempted: pull fully rebased and already tested.

rh-atomic-bot pushed a commit that referenced this pull request Sep 26, 2018
Prep for moving more of our parsing into Rust.  The main
thing here is that for JSON, we need to continue to ignore
unknown fields.  It took me a little while but I eventually
figured out that using `#[serde(flatten)]` works for this.
Seriously: serde is freaking amazingly awesome.

Closes: #1580
Approved by: jlebon
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.

3 participants