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] Fix translations dir discovery#16139
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
nicolas-grekas commentedOct 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 | - |
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.
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 containsaz andhe translations.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ok sorry, I merged really to quickly... let's try again
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.
@ogizanagi can you please work on this? (removing Resources/translations from Security/Core in 2.3 also)
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.
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 installsymfony/security andsymfony/security-core in parallel (see#16134) which led to theAuthenticationException 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@xabbuh : Why ? We should only remove those translations files from 2.3 and usesymfony/securityResources/translations folder.
Wouldn't be ok ? (it should not be propagated to upper branches)
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.
See#16141
…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