[FrameworkBundle] [Security] Remove trans from the security/core in 2.3 & dir loading #16141
Conversation
|
👍 |
|
Another solution is to load |
| $dirs[] = $translationsDir; | ||
| if (file_exists($translationsDir = dirname($r->getFilename()).'/../Resources/translations')) { | ||
| // with Symfony 2.4, the Security component was split into several subpackages | ||
| // and the translations have been moved to the symfony/security-core package |
There was a problem hiding this comment.
Actually, this comment is not 10000 completely right (we have translations in both packages). And by the way, having duplicated translations directories leads to inconsistent translations (for example, #15955 changed only one of the two translation files).
There was a problem hiding this comment.
Well, we have them outside of Core for BC reasons (using them is deprecated and they are removed in 3.0)
There was a problem hiding this comment.
Well, but then removing them from core in Symfony 2.3 is confusing for users imho if they should rely on them again after upgrading.
There was a problem hiding this comment.
Well, they should be no translation in security-core in 2.3 (they have been moved there in 2.4). the existing ones there in the 2.3 branch are mistakes when switching branch for merges IMO
|
You should not load both folders in the translator IMO. this would create more work for the loader by loading security translations twice, while the second one will be overwritten. What we should have to avoid missing translations in the new location is a unit test checking that both files are in sync in 2.4+ (actually 2.7+ because of maintained branches) |
|
Ok. I've just updated the PR. If the Otherwise, use |
|
👍 for this. @nicolas-grekas be careful when merging. the removal of the 2 translation files should be reverted when propagating the merge to newer branches |
|
👍 Status: Reviewed |
|
Thank you @ogizanagi. |
…ity/core in 2.3 & dir loading (ogizanagi) This PR was merged into the 2.3 branch. Discussion ---------- [FrameworkBundle] [Security] Remove trans from the security/core in 2.3 & dir loading | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #16139, #16133 | License | MIT | Doc PR | - See #16139 (comment). I think the most efficient solution is to remove translations from `Security/Core` in 2.3 only (should not be propagated to newest branches!) and load both folders if they exist. Commits ------- 1ed07a0 [FrameworkBundle] [Security] Remove trans from the security/core in 2.3 & dir loading
See #16139 (comment).
I think the most efficient solution is to remove translations from
Security/Corein 2.3 only (should not be propagated to newest branches!) and load both folders if they exist.