[FrameworkBundle] Fix translations dir discovery #16139
Conversation
nicolas-grekas
commented
Oct 6, 2015
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #16133 |
| License | MIT |
| Doc PR | - |
|
|
||
| if (file_exists($legacyTranslationsDir)) { | ||
| if (!file_exists($translationsDir) && file_exists($dir = dirname($r->getFilename()).'/../../Resources/translations')) { |
There was a problem hiding this comment.
Both path exists in 2.3, but only one contains every translation files :/
So in 2.3, only /../Resources/translations will be loaded, which only contains az and he translations.
There was a problem hiding this comment.
wait, we have the new path in 2.3 with only incomplete files ? This looks wrong to me.
There was a problem hiding this comment.
ok sorry, I merged really to quickly... let's try again
There was a problem hiding this comment.
@ogizanagi can you please work on this? (removing Resources/translations from Security/Core in 2.3 also)
There was a problem hiding this comment.
Indeed, I did not notice that translations are contained in both directories (even if not both are complete). I guess the real issue was caused by allowing to install symfony/security and symfony/security-core in parallel (see #16134) which led to the AuthenticationException being found first in the core package.
@nicolas-grekas Removing the translations from the core package means that we should use another class to detect the translations directory.
There was a problem hiding this comment.
@xabbuh : Why ? We should only remove those translations files from 2.3 and use symfony/security Resources/translations folder.
Wouldn't be ok ? (it should not be propagated to upper branches)
…s-grekas) This PR was merged into the 2.3 branch. Discussion ---------- [FrameworkBundle] Fix translations dir discovery | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16133 | License | MIT | Doc PR | - Commits ------- f37ceef [FrameworkBundle] Fix translations dir discovery
…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