[FrameworkBundle][Translation] Move translation compiler pass #22619

lepiaf · 2017-05-03T11:42:16Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	yes
Fixed tickets	part of #21284
License	MIT
Doc PR	n/a

move TranslationDumperPass, TranslationExtractorPass, TranslatorPass to Translation component.

stof

conflict rules are missing to ensure it does not get installed with an older version of symfony/translation (which would not allow registering it)

stof · 2017-05-03T11:54:15Z

UPGRADE-3.3.md

@@ -244,6 +244,18 @@ FrameworkBundle
   class has been deprecated and will be removed in 4.0. Use the
   `Symfony\Component\Workflow\DependencyInjection\ValidateWorkflowsPass` class instead.

+ * The `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\TranslationDumperPass`
+   class has been deprecated and will be removed in 4.0. Use the


this will have to go in a new UPGRADE-3.4.md file actually. It is too late to include this in 3.3 as we already released the beta

stof · 2017-05-03T11:54:21Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/TranslationDumperPass.php

-use Symfony\Component\DependencyInjection\Reference;
-use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+@trigger_error(sprintf('The %s class is deprecated since version 3.3 and will be removed in 4.0. Use Symfony\Component\Translation\DependencyInjection\TranslationDumperPass instead.', TranslationDumperPass::class), E_USER_DEPRECATED);


chalasr

The Translator component needs symfony/dependency-injection:~3.3 as dev requirement and a conflict for symfony/dependency-injection:<3.3. Also adding a test case for passes which miss one would be great.

chalasr · 2017-05-03T11:55:38Z

src/Symfony/Component/Translation/DependencyInjection/TranslationExtractorPass.php

+{
+    public function process(ContainerBuilder $container)
+    {
+        if (!$container->hasDefinition('translation.extractor')) {


the "collector" service id should be configurable through a constructor argument, same for the collected tag e.g. __construct($extractorServiceId = 'translation.extractor', $extractorTag = 'translation.extractor')

chalasr · 2017-05-03T11:55:54Z

src/Symfony/Component/Translation/DependencyInjection/TranslatorPass.php

+{
+    public function process(ContainerBuilder $container)
+    {
+        if (!$container->hasDefinition('translator.default')) {


same here (constructor)

chalasr · 2017-05-03T11:57:44Z

src/Symfony/Component/Translation/Tests/DependencyInjection/TranslatorPassTest.php

@@ -9,10 +9,10 @@
 * file that was distributed with this source code.
 */

-namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler;
+namespace Symfony\Component\Translation\Tests\DependencyInjection;


The original test case should be kept in FrameworkBundle with @group legacy and copied to the component, not moved.

chalasr · 2017-05-03T11:58:01Z

src/Symfony/Component/Translation/DependencyInjection/TranslationDumperPass.php

+{
+    public function process(ContainerBuilder $container)
+    {
+        if (!$container->hasDefinition('translation.writer')) {


same here (constructor)

chalasr · 2017-05-03T12:02:19Z

The fixed ticket would not be fixed actually, HttpKernel passes are still missing.

lepiaf · 2017-05-03T12:17:08Z

@chalasr I'm currently working in another pr to move HttpKernel passes

chalasr · 2017-05-03T12:32:33Z

src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

+ * Deprecated `TranslationExtractorPass`, use 
+    `Symfony\Component\Translation\DependencyInjection\TranslationExtractorPass` instead
+ * Deprecated `TranslatorPass`, use 
+     `Symfony\Component\Translation\DependencyInjection\TranslatorPass` instead


2 extra spaces before Symfony

chalasr · 2017-05-03T12:32:49Z

src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

+ * Deprecated `TranslationDumperPass`, use 
+   `Symfony\Component\Translation\DependencyInjection\TranslationDumperPass` instead
+ * Deprecated `TranslationExtractorPass`, use 
+    `Symfony\Component\Translation\DependencyInjection\TranslationExtractorPass` instead


1 extra space before Symfony

chalasr · 2017-05-03T12:34:00Z

@lepiaf thanks for working on this.

lepiaf · 2017-05-05T11:37:32Z

I did all requested changes. I will add new test for moved class.

chalasr

~3.4 can't be used as a constraint yet, that breaks tests. I would let it as is and wait for 3.3 to be stable/ 3.4 branch to be created

chalasr · 2017-05-07T08:41:36Z

src/Symfony/Component/Translation/composer.json

@@ -21,6 +21,7 @@
    },
    "require-dev": {
        "symfony/config": "~2.8|~3.0",
+        "symfony/dependency-injection": "~3.3",


you need to add a conflict rule in order to prevent errors when using 3.3 features (these passes use ones)

chalasr · 2017-05-07T08:46:26Z

src/Symfony/Component/Translation/DependencyInjection/TranslatorPass.php

+        $translatorServiceId = 'translator.default',
+        $loaderServiceId = 'translation.loader',
+        $loaderTag = 'translation.loader'
+    ) {


one-line signatures are preferred

chalasr · 2017-05-07T08:57:03Z

src/Symfony/Component/Translation/DependencyInjection/TranslatorPass.php

+     * @param string $translatorServiceId
+     * @param string $loaderServiceId
+     * @param string $loaderTag
+     */


phpdoc should be removed (same for properties, applies on the 3 passes), it doesn't give anything that an IDE couldn't guess

xabbuh · 2017-05-27T07:21:17Z

@lepiaf 3.3 and 3.4 branches are a thing now. Can you rebase here?

lepiaf · 2017-05-27T10:08:03Z

tests are failing but I don't know why. Is symfony/var-dumper missing in composer for frameworkbundle ?

chalasr

Build failure on deps=high is normal AFAIK. 👍

chalasr · 2017-06-21T12:35:01Z

ping deciders

fabpot · 2017-07-06T06:49:54Z

Thank you @lepiaf.

…er pass (lepiaf) This PR was merged into the 3.4 branch. Discussion ---------- [FrameworkBundle][Translation] Move translation compiler pass | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | part of #21284 | License | MIT | Doc PR | n/a move TranslationDumperPass, TranslationExtractorPass, TranslatorPass to Translation component. Commits ------- 74c951f [FrameworkBundle][Translation] Move translation compiler pass

carsonbot added Status: Needs Review FrameworkBundle Translation Feature Deprecation labels May 3, 2017

stof requested changes May 3, 2017

View reviewed changes

stof added this to the 3.4 milestone May 3, 2017

chalasr requested changes May 3, 2017

View reviewed changes

chalasr mentioned this pull request May 3, 2017

[FrameworkBundle] Move compiler passes to their component #21284

Closed

11 tasks

chalasr requested changes May 3, 2017

View reviewed changes

lepiaf changed the title ~~[FrameworkBundle][Translation] Move translation compiler pass~~ [WIP][FrameworkBundle][Translation] Move translation compiler pass May 5, 2017

chalasr requested changes May 7, 2017

View reviewed changes

lepiaf changed the title ~~[WIP][FrameworkBundle][Translation] Move translation compiler pass~~ [FrameworkBundle][Translation] Move translation compiler pass May 15, 2017

lepiaf changed the base branch from master to 3.4 May 22, 2017 11:02

chalasr approved these changes May 27, 2017

View reviewed changes

[FrameworkBundle][Translation] Move translation compiler pass

74c951f

fabpot approved these changes Jul 6, 2017

View reviewed changes

fabpot merged commit 74c951f into symfony:3.4 Jul 6, 2017

javiereguiluz mentioned this pull request Jul 16, 2017

Moved translation compiler pass symfony/symfony-docs#8165

Closed

This was referenced Oct 18, 2017

Release v3.4.0-BETA1 #24609

Merged

Release v4.0.0-BETA1 #24612

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle][Translation] Move translation compiler pass #22619

[FrameworkBundle][Translation] Move translation compiler pass #22619

lepiaf commented May 3, 2017 •

edited

Loading

stof left a comment

stof May 3, 2017

stof May 3, 2017

chalasr left a comment

chalasr May 3, 2017

chalasr May 3, 2017

chalasr May 3, 2017

chalasr May 3, 2017

chalasr commented May 3, 2017

lepiaf commented May 3, 2017

chalasr May 3, 2017 •

edited

Loading

chalasr May 3, 2017 •

edited

Loading

chalasr commented May 3, 2017

lepiaf commented May 5, 2017

chalasr left a comment

chalasr May 7, 2017 •

edited

Loading

chalasr May 7, 2017

chalasr May 7, 2017

xabbuh commented May 27, 2017

lepiaf commented May 27, 2017

chalasr left a comment

chalasr commented Jun 21, 2017

fabpot commented Jul 6, 2017

[FrameworkBundle][Translation] Move translation compiler pass #22619

[FrameworkBundle][Translation] Move translation compiler pass #22619

Conversation

lepiaf commented May 3, 2017 • edited Loading

stof left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr commented May 3, 2017

lepiaf commented May 3, 2017

chalasr May 3, 2017 • edited Loading

Choose a reason for hiding this comment

chalasr May 3, 2017 • edited Loading

Choose a reason for hiding this comment

chalasr commented May 3, 2017

lepiaf commented May 5, 2017

chalasr left a comment

Choose a reason for hiding this comment

chalasr May 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xabbuh commented May 27, 2017

lepiaf commented May 27, 2017

chalasr left a comment

Choose a reason for hiding this comment

chalasr commented Jun 21, 2017

fabpot commented Jul 6, 2017

lepiaf commented May 3, 2017 •

edited

Loading

chalasr May 3, 2017 •

edited

Loading

chalasr May 3, 2017 •

edited

Loading

chalasr May 7, 2017 •

edited

Loading