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

[2.0] Use storeAs=dbRef as the new default setting for references #1502

Merged
merged 1 commit into from
Jan 16, 2017

Conversation

coudenysj
Copy link
Contributor

As suggested in 30285e4, the recommended setting of storeAs=dbRef will become the default setting in 2.0.

@malarzm I had to update my composer.json locally to be able to run phpunit (see #1463).

@malarzm malarzm added the Task label Sep 30, 2016
@malarzm malarzm added this to the 2.0.0 milestone Sep 30, 2016
@malarzm
Copy link
Member

malarzm commented Sep 30, 2016

@coudenysj thanks for the PR! Overally looks good, you have some places with commented out code in the tests though, I think you wanted to remove them? :)

About the phpunit thing, thanks for reminding me once again, duly noted to change that finally.

@coudenysj
Copy link
Contributor Author

@malarzm The commented assertions are because I was not sure if I should remove the assertions or change the mapping information. I think I'll go with removing the assertions.

@alcaeus alcaeus force-pushed the odm-ng branch 2 times, most recently from b585acf to d0ff8b7 Compare January 13, 2017 07:00
@malarzm
Copy link
Member

malarzm commented Jan 14, 2017

@coudenysj would you mind rebasing this atop current odm-ng branch?

@coudenysj
Copy link
Contributor Author

@malarzm done

@malarzm
Copy link
Member

malarzm commented Jan 16, 2017

@coudenysj I think something went wrong as PR still has 11 commits instead of 2 you've made :)

@malarzm
Copy link
Member

malarzm commented Jan 16, 2017

Also if you could squash these two that would allow me to push the green button instead of doing the merge manually ;)

@coudenysj
Copy link
Contributor Author

The rebase was corrected, can't you merge pull requests with multiple commits?

@malarzm
Copy link
Member

malarzm commented Jan 16, 2017

I can (and often I do) but the 2nd commit doesn't bring anything to the history so there's no point in having it :)

@coudenysj
Copy link
Contributor Author

Correct. Done!

@malarzm
Copy link
Member

malarzm commented Jan 16, 2017

Thank you! \o/

@malarzm malarzm merged commit ca8d75d into doctrine:odm-ng Jan 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants