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

feat(codegen) skip_deserializing #265

Merged
merged 1 commit into from
Apr 12, 2016
Merged

feat(codegen) skip_deserializing #265

merged 1 commit into from
Apr 12, 2016

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Mar 7, 2016

Addresses part of #197.

This will be more exciting when I add logic to omit the T: Deserialize bound for fields that have skip_deserializing, like what #260 does for skip_serializing, but I am waiting for a code review on #260 before implementing that.

@dtolnay
Copy link
Member Author

dtolnay commented Mar 17, 2016

@erickt I see that you have been busy for the past 2 weeks but let me know if there is anything I can do to move forward any of my PRs.

@erickt
Copy link
Member

erickt commented Mar 17, 2016

@dtolnay: yeah I've been busy with work, but my head is popping up and I'm trying to catch up with my queue. I'll get to this today or tomorrow!

@dtolnay
Copy link
Member Author

dtolnay commented Apr 6, 2016

I have rebased to fix the conflict with #275.

@dtolnay
Copy link
Member Author

dtolnay commented Apr 11, 2016

I have rebased to fix the conflict with #281.

@erickt erickt merged commit 87393b6 into serde-rs:master Apr 12, 2016
@dtolnay dtolnay deleted the skip_deserializing branch April 12, 2016 16:39
@erickt
Copy link
Member

erickt commented Apr 12, 2016

@dtolnay: thanks for this as well! I merged it in. Have you begun work on getting this to work with #[default=...]? It appears to only work with Default::default at the moment.

@dtolnay
Copy link
Member Author

dtolnay commented Apr 12, 2016

No. I will work on it later this week, along with the equivalent of #260 so that fields we don't deserialize do not need to implement Deserialize.

@erickt
Copy link
Member

erickt commented Apr 12, 2016

Thanks @dtolnay! You're the best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants