[3.0][DoctrineBridge] Removed deprecated features #15900
Conversation
wouterj
commented
Sep 26, 2015
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | yes |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | - |
| License | MIT |
| Doc PR | - |
d539535 to
59f454d
Compare
| * | ||
| * @throws UnexpectedTypeException | ||
| */ | ||
| public function __construct($queryBuilder, $manager = null, $class = null) | ||
| public function __construct(QueryBuilder $queryBuilder) |
There was a problem hiding this comment.
this means that people will get a PHP fatal error instead of an UnexpectedTypeException, but I guess that is not a problem?
There was a problem hiding this comment.
not an issue: Symfony usage of this class is already validating the type earlier (in EntityType) anyway
|
tests are broken |
|
Status: needs work |
9ee907d to
349d928
Compare
|
Fixed. Status: Needs review |
|
@wouterj I don't agree with you. Half of the tests of the bridge are still throwing exceptions instead of running properly. Status: needs work |
349d928 to
26179d5
Compare
|
@stof agreed. Fixed some more things and made the tests pass locally. Should be ready (hopefully). Status: Needs review |
| @@ -140,7 +147,6 @@ public function configureOptions(OptionsResolver $resolver) | |||
| $options['em'], | |||
| $options['class'], | |||
| $qbParts, | |||
| $options['loader'], | |||
There was a problem hiding this comment.
as the loader is now returned from *Type::getLoader(), it'll always be the same. I don't think it makes sense to include it in the cache hash anymore.
This was also causing caching to become useless, as the object returned from getLoader() always was a new instance, resulting in another hash.
There was a problem hiding this comment.
it was not making caching useless, as $options['loader'] was set only in case the option was used explicitly (not the case by default.
I agree that the loader returned by getLoader does not belong to the cache key: the other parts of the cache key are what determine this loader, and this is why they are here
26179d5 to
f813e36
Compare
|
Fixed the comments |
|
Broken tests are not related to this PR. Anything else before merging? |
|
Afaik, no. |
|
Thank you @wouterj. |
…terJ) This PR was squashed before being merged into the 3.0-dev branch (closes #15900). Discussion ---------- [3.0][DoctrineBridge] Removed deprecated features | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- 6dc3cf9 [3.0][DoctrineBridge] Removed deprecated features