Skip to content

[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

Merged
merged 1 commit into from
Oct 27, 2018

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Oct 16, 2018

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

@@ -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');
Copy link
Member

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.

Copy link
Member Author

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)) {
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, check removed

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.2 Oct 20, 2018
@fabpot
Copy link
Member

fabpot commented Oct 26, 2018

@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.

@chalasr chalasr force-pushed the deprecate-old-translation-dirs branch 5 times, most recently from a2b95dc to 342c4a0 Compare October 26, 2018 19:18
@chalasr chalasr changed the title [FrameworkBundle] Deprecate old paths in translation commands [FrameworkBundle] Deprecate support for legacy translation and views directories Oct 26, 2018
@chalasr chalasr force-pushed the deprecate-old-translation-dirs branch from 342c4a0 to 78026ca Compare October 26, 2018 19:24
@chalasr chalasr force-pushed the deprecate-old-translation-dirs branch 9 times, most recently from 037ea64 to c4ed34a Compare October 26, 2018 21:14
@chalasr
Copy link
Member Author

chalasr commented Oct 26, 2018

@fabpot thanks for the ping, PR ready.

@chalasr chalasr changed the title [FrameworkBundle] Deprecate support for legacy translation and views directories [FrameworkBundle] Deprecate support for legacy directories in Translation comands Oct 26, 2018
@chalasr chalasr force-pushed the deprecate-old-translation-dirs branch 2 times, most recently from f7913b8 to 36ee73e Compare October 26, 2018 23:42
$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);
Copy link
Member

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 ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@chalasr chalasr force-pushed the deprecate-old-translation-dirs branch from 36ee73e to 5521f22 Compare October 27, 2018 10:35
@chalasr chalasr force-pushed the deprecate-old-translation-dirs branch 2 times, most recently from a45a9fc to afe797e Compare October 27, 2018 12:36
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);
Copy link
Member

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();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@chalasr chalasr force-pushed the deprecate-old-translation-dirs branch from afe797e to a7a8aad Compare October 27, 2018 14:37
@fabpot
Copy link
Member

fabpot commented Oct 27, 2018

Thank you @chalasr.

@fabpot fabpot merged commit a7a8aad into symfony:master Oct 27, 2018
fabpot added a commit that referenced this pull request Oct 27, 2018
…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
@chalasr chalasr deleted the deprecate-old-translation-dirs branch October 27, 2018 16:06
This was referenced Nov 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants