Adds generics annotations #104
Conversation
|
`findAll` and `findBy` should return `list<T>`, I think
Marco Pivetta
http://twitter.com/Ocramius
http://ocramius.github.com/
…On Fri, May 1, 2020 at 4:25 PM Philip Weinke ***@***.***> wrote:
Not sure if that's enough to close #72
<#72> but it's a start ;)
------------------------------
You can view, comment on, or merge this pull request online at:
#104
Commit Summary
- Adds generics annotations
File Changes
- *M* lib/Doctrine/Persistence/ManagerRegistry.php
<https://github.com/doctrine/persistence/pull/104/files#diff-70666339b628a1be48f066afe92d5c83>
(4)
- *M* lib/Doctrine/Persistence/ObjectManager.php
<https://github.com/doctrine/persistence/pull/104/files#diff-1fbc8e5ed87c0af8746757cffc4f4ce3>
(8)
- *M* lib/Doctrine/Persistence/ObjectRepository.php
<https://github.com/doctrine/persistence/pull/104/files#diff-b597361f77f1aa134bc59f970225621e>
(10)
Patch Links:
- https://github.com/doctrine/persistence/pull/104.patch
- https://github.com/doctrine/persistence/pull/104.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#104>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABFVEEXTUI64BQRUH2DNVLRPLLVFANCNFSM4MXEDZYQ>
.
|
|
Yeah, wasn't sure about that. It's a list in doctrine/orm, but doctrine/mongodb-odm uses the document id as array key. Not sure about the other odm implementations. I agree that it should be a list, though. Overriding the annotations in odm doesn't seem right but that's just my humble opinion. |
|
Nice PR! You gave me the motivation to setup Psalm on this lib! See #105 |
|
Do you have any opinions on this one, @alcaeus ? |
Sorry for the delay. Since ODM doesn't return a list but rather a struct, this has to be |
|
Note that this should go into 1.4.x since that version is supposed to have feature parity with 2.0. @weph could you please change the target branch and rebase? Thanks! |
|
Psalm support just landed on 1.4.x BTW :) |
|
I did the rebase for you. |
|
Reopening to get a report from Travis |
|
Thanks @weph ! |
|
Thank you, @greg0ire ! |
| @@ -111,9 +115,9 @@ public function flush(); | |||
| /** | |||
| * Gets the repository for a class. | |||
| * | |||
| * @param string $className | |||
There was a problem hiding this comment.
I think this line should be preserved to allow IDE know about required argument type.
There was a problem hiding this comment.
I agree. Do you mind creating a pull request?
Based on the changes upstream: doctrine/persistence#104 Refs #71
Not sure if that's enough to close #72 but it's a start ;)