Make doctrine/annotations an optional dependency #334
Conversation
W0rma
force-pushed
the
optional-annotations
branch
from
69ca1a0 to
2dd73d9
Compare
|
This looks nice, looking forward to it |
W0rma
force-pushed
the
optional-annotations
branch
from
fc17fa4 to
c4c3368
Compare
README.md
Outdated
| @@ -180,7 +191,7 @@ use JMS\Serializer\Annotation as Serializer; | |||
| use Hateoas\Configuration\Annotation as Hateoas; | |||
|
|
|||
| #[Serializer\XmlRoot('user')] | |||
| #[Hateoas\Relation('self', href: "expr('/api/users/' ~ object.getId())")] | |||
| #[Hateoas\Relation(name: 'self', href: "expr('/api/users/' ~ object.getId())")] | |||
There was a problem hiding this comment.
is the name argument needed? it seems that before, it was optional, making it look like annotations
There was a problem hiding this comment.
Thank you for the hint.
It is not necessary anymore after 9e8a232
| @@ -7,15 +7,9 @@ | |||
| use Hateoas\Configuration\Annotation as Hateoas; | |||
| use JMS\Serializer\Annotation as Serializer; | |||
|
|
|||
| /** | |||
| * @Serializer\ExclusionPolicy("all") | |||
There was a problem hiding this comment.
hmm., this change makes it mandatory to have the attribute driver, right? is this a BC break?
There was a problem hiding this comment.
Yes, I think so.
I just added the annotations again to avoid that possible BC break.
|
to me it looks good, i would just like to understand what happens is someone is using still annotations and uses some of the |
W0rma
force-pushed
the
optional-annotations
branch
from
c4c3368 to
880ec13
Compare
W0rma
force-pushed
the
optional-annotations
branch
from
880ec13 to
9e8a232
Compare
@goetas Thank you for the review - I addressed all your comments. After my latest adjustments the |
|
@W0rma does your implementation allow to use both annotations and attributes at the same time? If yes, do we have a testcase in which both drivers are enabled? |
@goetas Yes, it does. I added a test case which verifies that. |
|
lets ship it! |
Closes #333