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] Remove DiscriminatorField's name and fieldName #1478

Merged
merged 1 commit into from
Aug 5, 2016

Conversation

malarzm
Copy link
Member

@malarzm malarzm commented Jul 30, 2016

PR deprecating name and fixing our test suite will be sent soon (travis will be green again after #1479 is merged and this rebased)

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

alcaeus commented Jul 31, 2016

I assume this is because the discriminator field is not hydrated anyways? I've sometimes wondered why that is and thought it would be cool if the discriminator field value would be available in the document as well if we want. What do you think?

@malarzm
Copy link
Member Author

malarzm commented Jul 31, 2016

I believe this was done to make discriminator transparent (to keep people away from modifying, or trying to modify, it I reckon?) - it'd be good if @jmikola or @jwage would shed some light if they recall :) . Personally I prefer instanceof check so I'm not sure if there's value in exposing the field :)

@alcaeus
Copy link
Member

alcaeus commented Aug 1, 2016

That would make sense - it was in a previous project where it would have been helpful. Either way, let's remove it since it wasn't used anyways.

@malarzm malarzm force-pushed the remove-discriminator-fieldname branch from e6bcbca to d7004b1 Compare August 5, 2016 18:59
@malarzm malarzm merged commit a000a45 into doctrine:odm-ng Aug 5, 2016
@malarzm malarzm deleted the remove-discriminator-fieldname branch August 5, 2016 19:17
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.

2 participants