Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Cache] Deprecate DoctrineProvider#40908
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Cache] Deprecate DoctrineProvider#40908
Uh oh!
There was an error while loading.Please reload this page.
Conversation
derrabus commentedApr 22, 2021
cc@alcaeus |
8b209b9 tob912136Compare
alcaeus left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM, but we can allow cache 2.0, which will be released next week.
| "require-dev": { | ||
| "doctrine/annotations":"^1.10.4", | ||
| "doctrine/cache":"~1.0", | ||
| "doctrine/cache":"^1.11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This can also allow 2.0 as long as you're not using an actual implementation:
| "doctrine/cache":"^1.11", | |
| "doctrine/cache":"^1.11|^2.0", |
| "symfony/translation":"^4.4|^5.0", | ||
| "doctrine/annotations":"^1.10.4", | ||
| "doctrine/cache":"~1.0", | ||
| "doctrine/cache":"^1.11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Same as above with 2.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This needs more changes in the Validator component. Under certain circumstances, we still use an array cache from Doctrine. I'll do this in a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sounds good, thank you!
b912136 toc4c35d0CompareUh oh!
There was an error while loading.Please reload this page.
c4c35d0 to5fb1867Compare
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm also 👍 to add^2.0, either in this PR or in a follow up one.
Nyholm left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Would it be simpler if we also updated doctrine/annotations at the same time?
| $reader =newPsrCachedReader($this->annotationReader,$arrayAdapter,$this->debug); | ||
| }else { | ||
| $reader =newCachedReader($this->annotationReader,newDoctrineProvider($arrayAdapter),$this->debug); | ||
| $reader =newCachedReader($this->annotationReader, DoctrineProvider::wrap($arrayAdapter),$this->debug); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This line can be removed, right? It is dead code if we also update todoctrine/annotations:1.13.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As soon as we have a stable release of doctrine/annotations 1.13, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
doctrine/annotations 1.13 is not released yet and is not sure to be released before the next 5.x release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
oops sorry. I read the wrong version on packagist. I though it was released already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No worries. I also misread and it looks like this is scheduled for 5.4 anyways, so we can definitely make that change.
5fb1867 to262ac66Comparederrabus commentedMay 16, 2021
I'll update the PR as soon as#41230 is merged and has bubbled up to 5.x. |
262ac66 to0d0b932Compare0d0b932 to16fccfaComparefabpot commentedJul 4, 2021
Thank you@derrabus. |
The
DoctrineProviderclass has been reimplemented in thedoctrine/cachepackage, so we don't have to maintain it anymore.