-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Deprecate support for legacy directories in Translation comands #28892
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/Symfony/Bundle/FrameworkBundle/Tests/Command/TranslationUpdateCommandTest.php
Show resolved
Hide resolved
@@ -91,7 +91,6 @@ protected function setUp() | |||
{ | |||
$this->fs = new Filesystem(); | |||
$this->translationDir = sys_get_temp_dir().'/'.uniqid('sf_translation', true); | |||
$this->fs->mkdir($this->translationDir.'/Resources/translations'); | |||
$this->fs->mkdir($this->translationDir.'/Resources/views'); |
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.
We can remove this too, I guess.
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.
Yea, this PR needs work, will finish tomorrow :)
@@ -147,12 +155,24 @@ protected function execute(InputInterface $input, OutputInterface $output) | |||
if ($this->defaultTransPath) { | |||
$transPaths[] = $this->defaultTransPath.'/'.$bundle->getName(); | |||
} | |||
$transPaths[] = sprintf('%s/Resources/%s/translations', $kernel->getRootDir(), $bundle->getName()); | |||
$transPaths[] = $deprecatedPath = sprintf('%s/Resources/%s/translations', $kernel->getRootDir(), $bundle->getName()); | |||
if ($deprecatedPath !== $this->defaultTransPath.'/'.$bundle->getName() && is_dir($deprecatedPath)) { |
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 overriding path for bundles shouldn't match $this->defaultTransPath.'/'.$bundle->getName()
never, right?
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, check removed
@chalasr Will you have time to finish this PR? I'd like to wrap up 4.2 and this one is needed before the first beta. Thank you. |
a2b95dc
to
342c4a0
Compare
342c4a0
to
78026ca
Compare
037ea64
to
c4ed34a
Compare
@fabpot thanks for the ping, PR ready. |
f7913b8
to
36ee73e
Compare
$transPaths = array(); | ||
if (is_dir($dir = $rootDir.'/Resources/translations')) { | ||
if ($dir !== $this->defaultTransPath) { | ||
$notice = sprintf('Storing translations in the "%s" directory is deprecated since Symfony 4.2. ', $dir); |
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.
we are using single sentences for deprecation message: Storing ... since Symfony 4.2, use the ...
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.
fixed
36ee73e
to
5521f22
Compare
a45a9fc
to
afe797e
Compare
if (is_dir($dir = sprintf('%s/Resources/%s/translations', $rootDir, $bundle->getName()))) { | ||
$transPaths[] = $dir; | ||
$notice = sprintf('Storing translations files for "%s" in the "%s" directory is deprecated since Symfony 4.2, ', $dir, $bundle->getName()); | ||
@trigger_error($notice.($this->defaultTransPath ? sprintf('use the "%s" directory instead.', $this->defaultTransPath.'/bundles/'.$bundle->getName()) : sprintf('configure "translator.default_path" and create a "bundles/%s/" directory into it instead.', $bundle->getName())), E_USER_DEPRECATED); |
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.
Maybe same here and mention just $this->defaultTransPath
?
@@ -169,12 +206,20 @@ protected function execute(InputInterface $input, OutputInterface $output) | |||
if ($this->defaultTransPath) { | |||
$transPaths[] = $this->defaultTransPath.'/'.$bundle->getName(); | |||
} |
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.
Same here
afe797e
to
a7a8aad
Compare
Thank you @chalasr. |
…ries in Translation comands (chalasr) This PR was merged into the 4.2-dev branch. Discussion ---------- [FrameworkBundle] Deprecate support for legacy directories in Translation comands | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | todo Commits ------- a7a8aad [FrameworkBundle] Deprecate support for legacy translations and views directories
Uh oh!
There was an error while loading. Please reload this page.