Lots of automated migrations to Symfony Flex structure #8569

wouterj · 2017-10-30T22:46:41Z

Almost all of these changes need a check if the paragraphs surrounding the code block still make sense, but at least, it is a start.

I skipped the Best Pratices and Create Your Own Framework guides. Both of these need to be completely revisited.

Also, lots of other (configuration, bundles, controller and services) guides need to be checked carefully.

Please review and merge quickly @symfony/team-symfony-docs

fabpot · 2017-10-30T22:51:20Z

components/routing.rst

@@ -364,8 +364,7 @@ routes with UTF-8 characters:

    .. code-block:: php-annotations

-        // src/AppBundle/Controller/DefaultController.php


Would it be possible to keep such comments? We can probably automatically just strip the AppBundle directory. The new one would be src/Controller/DefaultController.php in that case.

Yes, I changed it everywhere, but we're in the components section, we shouldn't talk about framework conventions here. (probably, this needs to be extracted to its own subguide).

ok, makes sense.

yceruto · 2017-10-31T02:43:56Z

bundles/override.rst

@@ -26,7 +26,7 @@ Routing

 Routing is never automatically imported in Symfony. If you want to include
 the routes from any bundle, then they must be manually imported from somewhere
-in your application (e.g. ``app/config/routing.yml``).
+in your application (e.g. ``app/config/routes.yaml``).


config/routes.yaml

Reverted this change, the whole paragraph needs to be rewritten (and thus done in a non-automatic PR).

yceruto · 2017-10-31T02:46:49Z

console.rst

@@ -229,9 +227,9 @@ class. It uses special input and output classes to ease testing without a real
 console::

    // tests/AppBundle/Command/CreateUserCommandTest.php
-    namespace Tests\AppBundle\Command;
+    namespace Tests\App\Command;


App\Tests everywhere?

Fixed (every other occurence of this as well)

yceruto · 2017-10-31T03:03:02Z

form/form_dependencies.rst

@@ -105,23 +105,23 @@ manually and tag it with ``form.type``:

    .. code-block:: yaml

-        # src/AppBundle/Resources/config/services.yml
+        # src/Resources/config/services.yaml


It should refer to config/services.yaml? same for other formats.

yceruto · 2017-10-31T03:10:16Z

http_cache.rst

    use Symfony\Component\HttpFoundation\Request;

    // ...
-    $kernel = new AppKernel('prod', false);
+    $kernel = new Kernel('prod', false);
    $kernel->loadClassCache();


this line is obsolete?

Reverted, all front controller and kernel changes should be done manually.

yceruto · 2017-10-31T03:36:15Z

controller/argument_value_resolver.rst

@@ -148,14 +148,14 @@ and adding a priority.

    .. code-block:: yaml

-        # app/config/services.yml
+        # app/config/services.yaml


config/* everywhere?

Done for all services and routes files, the remaining onces need all be visited manually.

yceruto · 2017-10-31T03:36:53Z

deployment/azure-website.rst

@@ -294,7 +294,7 @@ below:

 The code of the Symfony application has now been deployed to the Azure Website
 which you can browse from the file explorer of the Kudu application. You should
-see the ``app/``, ``src/`` and ``web/`` directories under your ``site/wwwroot``
+see the ``app/``, ``src/`` and ``public/`` directories under your ``site/wwwroot``


"see the ``config/``, ..."

yceruto · 2017-10-31T03:49:42Z

http_cache/esi.rst

@@ -126,7 +126,7 @@ matter), Symfony uses the standard ``render`` helper to configure ESI tags:

    .. code-block:: twig

-        {# app/Resources/views/static/about.html.twig #}
+        {# templates/static/about.html.twig #}

        {# you can use a controller reference #}
        {{ render_esi(controller('AppBundle:News:latest', { 'maxPerPage': 5 })) }}


App\Controller\NewsController::latestAction use FQNC notation everywhere?

Let's change that in a manual PR as well (not so easy to automate)

yceruto · 2017-10-31T03:50:44Z

security/csrf_in_login_form.rst

@@ -132,7 +132,7 @@ using the login form:

    .. code-block:: html+twig

-        {# src/AppBundle/Resources/views/Security/login.html.twig #}
+        {# src/Resources/views/Security/login.html.twig #}


maybe this templates should be moved to templates/?

yceruto · 2017-10-31T03:52:25Z

security/custom_authentication_provider.rst

@@ -459,10 +459,10 @@ to service ids that may not exist yet: ``AppBundle\Security\Authentication\Provi
 Now that your services are defined, tell your security context about your
 factory in your bundle class::

-    // src/AppBundle/AppBundle.php
+    // src/AppBundle.php
    namespace AppBundle;


both lines should be updated, probably this code lives now in Kernel.php.

wouterj · 2017-10-31T10:49:36Z

Thanks a lot for your detailed review @yceruto ! I've now fixed everything or reverted the change as they have to be done in another PR (to not make this too big and hard to merge).

yceruto

Awesome!

yceruto · 2017-10-31T12:35:44Z

service_container.rst

@@ -1067,5 +1067,5 @@ Learn more
    /service_container/*

 .. _`service-oriented architecture`: https://en.wikipedia.org/wiki/Service-oriented_architecture
-.. _`Symfony Standard Edition (version 3.3) services.yaml`: https://github.com/symfony/symfony-standard/blob/3.3/app/config/services.yaml
+.. _`Symfony Standard Edition (version 3.3) services.yaml`: https://github.com/symfony/symfony-standard/blob/3.3/config/services.yaml


should be reverted or referenced to the recipe instead?

wouterj · 2017-10-31T21:44:26Z

Applied the comment and also fixed the build

yceruto · 2017-10-31T21:58:46Z

security/custom_authentication_provider.rst


    use App\DependencyInjection\Security\Factory\WsseFactory;
-    use Symfony\Component\HttpKernel\Bundle\Bundle;
-    use Symfony\Component\DependencyInjection\ContainerBuilder;


add use Symfony\Component\HttpKernel\Kernel as BaseKernel?

weaverryan · 2017-11-02T01:05:12Z

YES, awesome! Thank you Wouter! Thanks to the review from @yceruto, I'm happy to merge this quickly. If we find any bugs, we can make sure to automate those "fixes" across everything. So I think merging this right now is a great idea :)

…(wouterj) This PR was squashed before being merged into the master branch (closes #8569). Discussion ---------- Lots of automated migrations to Symfony Flex structure Almost all of these changes need a check if the paragraphs surrounding the code block still make sense, but at least, it is a start. I skipped the Best Pratices and Create Your Own Framework guides. Both of these need to be completely revisited. Also, lots of other (configuration, bundles, controller and services) guides need to be checked carefully. Please review and merge quickly @symfony/team-symfony-docs Commits ------- 57baceb Fixed build failures 6853f61 Some manual tweaks to the automated migrations 9e5cd17 Lots of automated migrations to Symfony Flex structure

This PR was merged into the master branch. Discussion ---------- Migrating Tests topics to Symfony Flex structure #8569 continuation. I've excluded `creating-the-project.rst` guide, because it needs to be changed completely. Commits ------- 9dd5a9d Migrating Tests topics to Symfony Flex structure

Lots of automated migrations to Symfony Flex structure

9e5cd17

carsonbot added the Status: Needs Review label Oct 30, 2017

fabpot reviewed Oct 30, 2017

View reviewed changes

yceruto reviewed Oct 31, 2017

View reviewed changes

wouterj added 2 commits October 31, 2017 22:36

Some manual tweaks to the automated migrations

6853f61

Fixed build failures

57baceb

wouterj force-pushed the automated-sf3.4-updates branch from 7064286 to 57baceb Compare October 31, 2017 21:44

yceruto reviewed Oct 31, 2017

View reviewed changes

weaverryan merged commit 57baceb into symfony:master Nov 2, 2017

yceruto mentioned this pull request Nov 2, 2017

Migrating Tests topics to Symfony Flex structure #8577

Merged

		@@ -364,8 +364,7 @@ routes with UTF-8 characters:

		.. code-block:: php-annotations

		// src/AppBundle/Controller/DefaultController.php

Uh oh!

Lots of automated migrations to Symfony Flex structure #8569

Lots of automated migrations to Symfony Flex structure #8569

Uh oh!

Conversation

wouterj commented Oct 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wouterj commented Oct 31, 2017

Uh oh!

yceruto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wouterj commented Oct 31, 2017

Uh oh!

yceruto Oct 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weaverryan commented Nov 2, 2017

Uh oh!

Uh oh!

yceruto Oct 31, 2017 •

edited

Loading