Skip to content

PHPC-550: Always encode ODS field when serializing Persistable documents #226

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

Merged
merged 5 commits into from
Feb 22, 2016

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Feb 10, 2016

@jmikola jmikola force-pushed the phpc-550 branch 3 times, most recently from 799d382 to aff4943 Compare February 12, 2016 21:16
@jmikola jmikola changed the title [WIP] PHPC-550: Always encode ODS field when serializing Persistable documents PHPC-550: Always encode ODS field when serializing Persistable documents Feb 12, 2016
protected $streetAddress;
protected $city;
protected $postalCode;
use MongoDB\BSON as BSON;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't change test cases! I realize it no longer works, and hence the XFAIL is fine, but I don't see why the test itself should be so massively changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the refactoring commit.

The modified ODS tests return atomic modifiers through bsonSerialize(), which conflicts with __pclass injection (the resulting newObj is neither an update nor a replacement document). Rather than delete these tests, we'll mark them as expecting failure until we allow such functionality in another interface down the line.
@derickr
Copy link
Contributor

derickr commented Feb 15, 2016

LGTM

@jmikola jmikola merged commit ba83c54 into mongodb:master Feb 22, 2016
jmikola added a commit that referenced this pull request Feb 22, 2016
@jmikola jmikola deleted the phpc-550 branch February 22, 2016 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants