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

Don't crash the deserializer if object has an invalid class set #29

Merged
merged 1 commit into from Aug 11, 2017

Conversation

Projects
None yet
2 participants
@jefshe
Contributor

jefshe commented Aug 1, 2017

Relates to #28

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Aug 1, 2017

Owner

The PR fails on the unit tests, and that is a no-no.

Owner

eddelbuettel commented Aug 1, 2017

The PR fails on the unit tests, and that is a no-no.

@jefshe

This comment has been minimized.

Show comment
Hide comment
@jefshe

jefshe Aug 1, 2017

Contributor

fixed now, made a few too many assumptions about the class attribute

Contributor

jefshe commented Aug 1, 2017

fixed now, made a few too many assumptions about the class attribute

@jefshe

This comment has been minimized.

Show comment
Hide comment
@jefshe

jefshe Aug 11, 2017

Contributor

@eddelbuettel, could I get your thoughts on the new PR? I just added a catch condition to handle this specific case.

Contributor

jefshe commented Aug 11, 2017

@eddelbuettel, could I get your thoughts on the new PR? I just added a catch condition to handle this specific case.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Aug 11, 2017

Owner

Truly sorry, caught that then ten days ago and was side-tracked, and it got overlooked.

Will take another look.

Owner

eddelbuettel commented Aug 11, 2017

Truly sorry, caught that then ten days ago and was side-tracked, and it got overlooked.

Will take another look.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Aug 11, 2017

Owner

Looks fine to me. Anybody else want to comment?

Owner

eddelbuettel commented Aug 11, 2017

Looks fine to me. Anybody else want to comment?

@eddelbuettel eddelbuettel merged commit d0e8838 into eddelbuettel:master Aug 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment