[Translator] deprecate getMessages in favor of getCatalogue. #14546
Conversation
aitboudad
commented
May 4, 2015
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Fixed tickets | #14529 |
| Tests pass? | yes |
| License | MIT |
23161ad to
c3567f8
Compare
| @@ -292,6 +292,8 @@ protected function getLoaders() | |||
| */ | |||
| public function getMessages($locale = null) | |||
| { | |||
| trigger_error('The '.__METHOD__.' method is deprecated since version 2.8 and will be removed in 3.0. Use the getCatalogue() method instead.', E_USER_DEPRECATED); | |||
There was a problem hiding this comment.
I would say Rely on the TranslatorBagInterface instead. The current message saying Use the getCatalogue() method instead is confusing because getCatalogue is not a drop-in replacement for getMessages (it is the more powerful way to implement the feature though)
|
You need to update the UPGRADE-2.8.md file too, to describe the migration path. |
c3567f8 to
267cf79
Compare
267cf79 to
6cb4f03
Compare
|
@stof I deprecated loadCatalogue too |
6cb4f03 to
472f8b7
Compare
| $this->initializeCatalogue($locale); | ||
| } else { | ||
| $this->initializeCacheCatalogue($locale); | ||
| } |
There was a problem hiding this comment.
This change is not BC. People overwriting loadCatalogue would miss their extended behavior after this change (and this is why protected stuff are a pain for BC)
472f8b7 to
b7035d4
Compare
| @@ -311,6 +313,8 @@ public function getMessages($locale = null) | |||
| */ | |||
| protected function loadCatalogue($locale) | |||
| { | |||
| trigger_error('The '.__METHOD__.' method is deprecated since version 2.8 and will be removed in 3.0. Rely on the TranslatorBagInterface instead', E_USER_DEPRECATED); | |||
There was a problem hiding this comment.
this now triggers deprecations in Symfony itself, which is bad.
what you need to do is to trigger the deprecation in caller places in case the method is overwritten (have a look at the way we did for setDefaultOptions in form types, or for Kernel::init even though this one was simpler as the default method does nothing)
There was a problem hiding this comment.
I suggest keeping this PR focused on getMessages only and deal with loadCatalogue later once we have a replacement for the extension point
b7035d4 to
03beebb
Compare
|
👍 |
| { | ||
| $this->iniSet('error_reporting', -1 & ~E_USER_DEPRECATED); |
There was a problem hiding this comment.
is this still required? I've seen tests without it and sometimes a @group legacy annotation. What exactly is the workflow?
There was a problem hiding this comment.
Using the legacy group is sufficient and I suggest to only use that way.
|
@aitboudad I don't see where you deprecate the |
03beebb to
2869a32
Compare
|
👍 |
…logue. (aitboudad) This PR was merged into the 2.8 branch. Discussion ---------- [Translator] deprecate getMessages in favor of getCatalogue. | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Fixed tickets | #14529 | Tests pass? | yes | License | MIT Commits ------- 2869a32 [Translator] deprecate getMessages in favor of getCatalogue.