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

Deprecate field type annotation classes in favor of Field #1318

Closed
wants to merge 4 commits into from
Closed

Deprecate field type annotation classes in favor of Field #1318

wants to merge 4 commits into from

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Dec 19, 2015

Closes #1207.

Starting with PHP 7, some of the type annotation ODM uses are reserved keywords:

  • int
  • string
  • bool
  • float

To avoid having large inconsistencies we decided to deprecate all type annotations (e.g. those extending AbstractField and only overwriting the type property) to be removed in ODM 2.0. This affects the following annotation classes:

  • Bin
  • BinCustom
  • BinFunc
  • BinMD5
  • BinUUID
  • BinUUIDRFC4122
  • Bool
  • Boolean
  • Collection
  • Date
  • Float
  • Hash
  • Increment
  • Int
  • Integer
  • Key
  • ObjectId
  • Raw
  • String
  • Timestamp

Instead, users should switch to using @Field(type="...") to map fields.

@alcaeus alcaeus added this to the 1.1 milestone Dec 19, 2015
@malarzm
Copy link
Member

malarzm commented Dec 23, 2015

I think this among the others from bigger list should have different notes?

Beside that big 👍 for going through everything!

@alcaeus
Copy link
Member Author

alcaeus commented Dec 23, 2015

Yes, we could expand the wording, should we also add an example of how the field show be mapped?

@malarzm
Copy link
Member

malarzm commented Dec 23, 2015

should we also add an example of how the field show be mapped?

Was thinking about it and frankly I'm not sure... existing examples are updated for the new way and we could use that until absolute removal which will need documentation changes anyway so we could postpone writing new chapter? Unless you feel like doing that :D

@alcaeus
Copy link
Member Author

alcaeus commented Dec 23, 2015

I'm not sure we need this. I think for 1.1 just deprecating the classes is a good first step. We can then always throw deprecation notices in AnnotationDriver when a deprecated annotation is used.

@malarzm
Copy link
Member

malarzm commented Dec 23, 2015

👍

@malarzm
Copy link
Member

malarzm commented Jan 7, 2016

Another 👍 after rewording deprecation notes :)

@@ -19,7 +19,10 @@

namespace Doctrine\ODM\MongoDB\Mapping\Annotations;

/** @Annotation */
/**
* @Annotation
Copy link
Member

Choose a reason for hiding this comment

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

Is @Annotation a convention from Doctrine ORM or Symfony? I'm curious if we should just replace this with a one-line class description (e.g. "Annotation for binary data field").

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC, the @Annotation annotation tells the annotation reader that the class in question can be used as an annotation.
I'm actually 👍 to documenting annotation classes and their properties while we're at it.

@jmikola
Copy link
Member

jmikola commented Jan 7, 2016

One question above, but LGTM otherwise. Thanks for the tedious search/replace job!

@@ -19,7 +19,11 @@

namespace Doctrine\ODM\MongoDB\Mapping\Annotations;

/** @Annotation */
/**
* Identifies a class as document that can be stored in the database
Copy link
Member

Choose a reason for hiding this comment

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

"as a document"

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

Successfully merging this pull request may close these issues.

3 participants