Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle][Validator] Fix apc cache service & config#16795
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
The previous deprecated notice says, the class ApcCache will be removed, not that the alias "APC" will be discontinued.
This is a new BC break
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.
Three options then:
- trigger a deprecation in 2.8, now released ? (as a "bugfix": a missing deprecation notice)
- keep the ability to pass "apc", but trigger a new deprecation in 3.0. Then, remove this ability in 3.1
- keep the ability to pass "apc"
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.
Given Symfony 3 had been released, the options 1 is not valid
Given it's a BC break, the option 2 is not valid. => Have to wait Symfony 4
IMO option 3 is the only one available
ping@nicolas-grekas
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.
@jderusse I don't agree. There is no BC break in 3.0 if we remove the apc logic. Configuring apc in 3.0 does not work anyway.ApcCache does not exist anymore.
So we should go with option 1 as planned: Add missing deprecation in 2.8 and removevalidator.mapping.cache.apc service in 3.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.
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.
Removing the feature because it's not documented, mays be a point.
BUT, If someone use the configurationcache: apc with symfony 2.7, he never been warned about the deprecation. Upgrading to Symfony 3 lead to an error due to the missing class.
He may expect we fix the bug, instead of removing the feature.
Just my 2 cents and playing the devil's advocate. I understand the point, to cleanup this code 😉
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.
In order to upgrade from symfony 2.7 to 3.0, he'll need to update to 2.8 first. Then adding the deprecation in 2.8 in a bugfix is the right thing to do, and no one should expect having this service anymore in 3.0.
I removed the BC flag from the PR description accordingly. The existence of thevalidator.mapping.cache.apc service and the removal of thevalidator.mapping.cache.doctrine.apc one in 3.0 (as well as its deprecation in 2.8 instead of first one) is a bug and should be fixed in both branches.
Tobion commentedDec 3, 2015
Status: Needs Work |
jderusse commentedDec 3, 2015
Notice that service |
ogizanagi commentedDec 3, 2015
Yes, we can also notice that the |
dunglas commentedDec 7, 2015
👍 but the As the serializer and the validator only support Doctrine Cache, I don't see what the |
jakzal commentedDec 8, 2015
Something went wrong here. The |
ogizanagi commentedDec 8, 2015
@jakzal : Yes, it's exactly what happened. But let's forget about this PR for now.
What bothers me however is to deprecate a service in the same version it appeared. Maybe we should only fix the deprecation on the right service in 2.8, and then deprecate |
dunglas commentedDec 10, 2015
👍 to merge this one. |
dunglas commentedDec 15, 2015
Any update regarding this PR? This is a blocker to use Symfony 3 in prod. /cc @symfony/deciders |
xabbuh commentedDec 15, 2015
I would be in favour of merging this one and trigger a deprecation in 2.8. |
ogizanagi commentedDec 15, 2015
I suggest removing the second commit in#16822 (I'll do it right now) and merge it first. The deprecation should be fixed in 2.8 anyway. Then merge this one if necessary, but everything should be properly updated during the merge of 2.8 into 3.0. |
jonathanbull commentedDec 27, 2015
Any update regarding this PR? It looks like a fresh Symfony project still features an invalid |
wouterj commentedDec 27, 2015
Isn't it better to just quickly fix this error by merging this or mine PRs and then later decide on whether more actions should be taken? Currently, Symfony 3 with validator mapping cache just doesn't work as it should: bad dev experience... |
peterrehm commentedJan 15, 2016
Any update on this? |
…tion (ogizanagi)This PR was merged into the 2.8 branch.Discussion----------[FrameworkBundle][Validator] Fix apc cache service deprecation| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Related to#16795I guess the deprecation was on the wrong service.Also, no deprecation notice was triggered about using `"apc"` as the value of the `framework.validation.cache` configuration option. This PR adds the missing deprecation.> 📝 _NOTE_: The standard edition will need to be updated [here](https://github.com/symfony/symfony-standard/blob/2.8/app/config/config_prod.yml#L6).Commits-------907bbec [FrameworkBundle][Validator] Fix apc cache service deprecation
ogizanagi commentedJan 25, 2016
fabpot commentedJan 25, 2016
Thank you@ogizanagi. |
…g (ogizanagi)This PR was merged into the 3.0 branch.Discussion----------[FrameworkBundle][Validator] Fix apc cache service & config| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#16793| License | MIT| Doc PR | -_Keep track of#16794 (comment)_NOTE_: This PR is on standby. If#16822 is merged, this one might probably be closed, as everything will be done during the merge.Commits-------94a1728 [FrameworkBundle][Validator] Fix apc cache service & config
Keep track of#16794 (comment)
NOTE: This PR is on standby. If#16822 is merged, this one might probably be closed, as everything will be done during the merge.