Removed deprecated features and notices #8622

javiereguiluz · 2017-11-11T17:16:16Z

@xabbuh and @weaverryan to avoid a nightmare or rebases and problems, could we please give top priority to merging this PR? Thanks! It doesn't have to be perfect or 100% completed. We can do more fixes in another PR. Thanks!

Some articles explain things that are removed. Instead of removing those articles entirely and to avoid user confusion, I propose to keep those articles but explain that those features no longer exist. For example, security/acl.rst now contains the following:

.. index::
   single: Security; Access Control Lists (ACLs)

How to Use Access Control Lists (ACLs)
======================================

.. caution::

    ACL support was removed in Symfony 4.0. Install the `Symfony ACL bundle`_
    and refer to its documentation if you want to keep using ACL.

.. _`Symfony ACL bundle`: https://github.com/symfony/acl-bundle

Articles like this one:

security/acl.rst
security/acl_advanced.rst
components/event_dispatcher/container_aware_dispatcher.rst

We can remove all these deprecated articles in 4.1

@iltar could you please create a PR to update the changes related to "controller arguments" in these two articles? Thanks!

components/http_kernel.rst
create_framework/http_kernel_controller_resolver.rst

I don't understand the changes well (ControllerResolverInterface removed getArguments())

@HeahDude could you please create a PR to update the changes needed about the deprecation of using false in property_path instead of mapped in reference/forms/types/options/property_path.rst.inc? Thanks!

There is a pending deprecation message in security/multiple_user_providers.rst ... but that's fixed in the pending PR #8612.

A final question: I've removed the explanation of the deprecated kernel.root_dir in reference/configuration/kernel.rst and kept only kernel.project_dir. Would you instead prefer to keep a reference to the still heavily used kernel.root_dir to explain that it's deprecated?

linaori · 2017-11-11T17:34:29Z

I'll take a look on Monday

wouterj

Left comments, but can be done after merging.

wouterj · 2017-11-12T11:43:26Z

bundles/extension.rst

@@ -129,17 +129,10 @@ read more about it, see the ":doc:`/bundles/configuration`" article.
 Adding Classes to Compile
 -------------------------

-.. versionadded:: 3.3
-    This technique is discouraged and the ``addClassesToCompile()`` method was
-    deprecated in Symfony 3.3 because modern PHP versions make it unnecessary.


Unless I'm wrong, this versionadded says "Don't use what's descriped below." (so it's more a .. caution:: than versionadded). This means the article should be removed (as the feature got removed in Symfony 4).

I though the same ... but only addClassesToCompile() is deprecated/removed. addAnnotatedClassesToCompile() is still valid and used in Sf4.

wouterj · 2017-11-12T11:45:13Z

bundles/override.rst

-third-party bundles without using :doc:`/bundles/inheritance`, which is
-deprecated since Symfony 3.4.
+third-party bundles without using :doc:`/bundles/inheritance`, which was
+removed in Symfony 4.0.


Let's make this description generic to not require any more updates in later versions (and thus merge conflicts), e.g:

When using another bundle, you might want to customize or override some of its features. This document describes ways of overriding the most common features of a bundle.

wouterj · 2017-11-12T11:45:57Z

components/console/events.rst


 Listeners receive a
 :class:`Symfony\\Component\\Console\\Event\\ConsoleExceptionEvent` event::

    use Symfony\Component\Console\Event\ConsoleExceptionEvent;
    use Symfony\Component\Console\ConsoleEvents;

-    $dispatcher->addListener(ConsoleEvents::EXCEPTION, function (ConsoleExceptionEvent $event) {
+    $dispatcher->addListener(ConsoleEvents::ERROR, function (ConsoleExceptionEvent $event) {


Typehint should be ConsoleErrorEvent

wouterj · 2017-11-12T11:46:33Z

components/console/events.rst

@@ -117,19 +114,6 @@ Listeners receive a
        $event->setException(new \LogicException('Caught exception', $exitCode, $event->getException()));


$event->getError()

wouterj · 2017-11-12T11:47:46Z

components/event_dispatcher/container_aware_dispatcher.rst

-        }
-    }
+    The ``ContainerAwareEventDispatcher`` was removed in Symfony 4.0. Use
+    ``EventDispatcher`` with closure-proxy injection instead.


reference article/section about closure-proxy injection?

I don't understand this comment. Is there an article that we could link to in this comment? Or are you suggesting to create a new article? Thanks!

wouterj · 2017-11-12T11:49:23Z

profiler/matchers.rst

-            ),
-        ));
+    The possibility to use a matcher to enable the profiler conditionally was
+    removed in Symfony 4.0.


We should add information about doing $profiler->disable() in your controller explicitly if you need this functionality.

weaverryan · 2017-11-12T20:31:41Z

+1 for removing kernel.root_dir entirely. It will be easy for people to find it on the internet anyways.

@xabbuh

… weaverryan) This PR was merged into the master branch. Discussion ---------- Removed deprecated features and notices @xabbuh and @weaverryan to avoid a nightmare or rebases and problems, could we please give top priority to merging this PR? Thanks! It doesn't have to be perfect or 100% completed. We can do more fixes in another PR. Thanks! ----- Some articles explain things that are removed. Instead of removing those articles entirely and to avoid user confusion, I propose to keep those articles but explain that those features no longer exist. For example, `security/acl.rst` now contains the following: ```rst .. index:: single: Security; Access Control Lists (ACLs) How to Use Access Control Lists (ACLs) ====================================== .. caution:: ACL support was removed in Symfony 4.0. Install the `Symfony ACL bundle`_ and refer to its documentation if you want to keep using ACL. .. _`Symfony ACL bundle`: https://github.com/symfony/acl-bundle ``` Articles like this one: * security/acl.rst * security/acl_advanced.rst * components/event_dispatcher/container_aware_dispatcher.rst We can remove all these deprecated articles in 4.1 ----- @iltar could you please create a PR to update the changes related to "controller arguments" in these two articles? Thanks! * `components/http_kernel.rst` * `create_framework/http_kernel_controller_resolver.rst` I don't understand the changes well (`ControllerResolverInterface` removed `getArguments()`) ----- @HeahDude could you please create a PR to update the changes needed about the deprecation of using `false` in `property_path` instead of `mapped` in `reference/forms/types/options/property_path.rst.inc`? Thanks! ----- There is a pending deprecation message in `security/multiple_user_providers.rst` ... but that's fixed in the pending PR #8612. ----- A final question: I've removed the explanation of the deprecated `kernel.root_dir` in `reference/configuration/kernel.rst` and kept only `kernel.project_dir`. Would you instead prefer to keep a reference to the still heavily used `kernel.root_dir` to explain that it's deprecated? Commits ------- 02c676a fixing old reference eb53f14 Fixing test failure 47a224a Fixed ater Wouter's review 597b75b Removed more references about profiler.matcher option 0266046 Removed deprecated features and notices

weaverryan · 2017-11-12T20:43:13Z

I've just merged this so we can keep conflicts low and keep moving quickly (thanks Wouter for the fast review). I did get the tests passing.

If anyone does see any issues, please open a separate PR!

@javiereguiluz can you open separate issues on the 4.0 milestone for the other things that you needed done by other people?

Thanks!

javiereguiluz · 2017-11-12T20:53:47Z

Done! Thanks for your great and quick reviews and for merging this. Cheers!

Removed deprecated features and notices

0266046

carsonbot added the Status: Needs Review label Nov 11, 2017

Removed more references about profiler.matcher option

597b75b

wouterj approved these changes Nov 12, 2017

View reviewed changes

wouterj added Status: Reviewed and removed Status: Needs Review labels Nov 12, 2017

wouterj added this to the 4.0 milestone Nov 12, 2017

javiereguiluz and others added 2 commits November 12, 2017 17:54

Fixed ater Wouter's review

47a224a

Fixing test failure

eb53f14

fixing old reference

02c676a

weaverryan merged commit 02c676a into symfony:master Nov 12, 2017

This was referenced Nov 12, 2017

Document the removal of getArguments() from ControllerResolverInterface #8626

Closed

Remove the deprecated behavior of property_path in form types #8627

Closed

javiereguiluz mentioned this pull request Dec 23, 2017

[DI] Made container parameters case sensitive #8359

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Removed deprecated features and notices #8622

Removed deprecated features and notices #8622

Uh oh!

javiereguiluz commented Nov 11, 2017 •

edited

Loading

Uh oh!

linaori commented Nov 11, 2017

Uh oh!

wouterj left a comment

Uh oh!

wouterj Nov 12, 2017

Uh oh!

javiereguiluz Nov 12, 2017

Uh oh!

wouterj Nov 12, 2017

Uh oh!

wouterj Nov 12, 2017

Uh oh!

wouterj Nov 12, 2017

Uh oh!

wouterj Nov 12, 2017

Uh oh!

javiereguiluz Nov 12, 2017

Uh oh!

wouterj Nov 12, 2017

Uh oh!

weaverryan commented Nov 12, 2017

Uh oh!

weaverryan commented Nov 12, 2017

Uh oh!

javiereguiluz commented Nov 12, 2017

Uh oh!

Uh oh!

		@@ -117,19 +114,6 @@ Listeners receive a
		$event->setException(new \LogicException('Caught exception', $exitCode, $event->getException()));

Uh oh!

Removed deprecated features and notices #8622

Removed deprecated features and notices #8622

Uh oh!

Conversation

javiereguiluz commented Nov 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linaori commented Nov 11, 2017

Uh oh!

wouterj 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!

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!

weaverryan commented Nov 12, 2017

Uh oh!

weaverryan commented Nov 12, 2017

Uh oh!

javiereguiluz commented Nov 12, 2017

Uh oh!

Uh oh!

javiereguiluz commented Nov 11, 2017 •

edited

Loading