NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.7k

[FrameworkBundle] Add service and alias deprecation message to debug:container <name> output#46814

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:6.2from94noni:debug-container-deprecated

Jul 21, 2022

Merged

[FrameworkBundle] Add service and alias deprecation message to debug:container <name> output#46814

fabpot merged 1 commit intosymfony:6.2from94noni:debug-container-deprecated

Jul 21, 2022

Conversation

Copy link

Contributor

94noni commentedJun 30, 2022•
edited
Loading

Q	A
Branch?	6.2
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	.
License	MIT
Doc PR	.

Hi, thedebug:container is great to understand container's services in the app

Having thisdeprecated warning can be valuable in such debug context, that is why I propose to add this on the output

For ex with such definition:

        ->set('url_helper', UrlHelper::class)            ->args([service('request_stack'),service('router.request_context')->ignoreOnInvalid(),            ])            ->deprecate('symfony/framework-bundle','6.2','The service "%service_id% is deprecated. Use "FooBarService" instead.')        ->alias(UrlHelper::class,'url_helper')            ->deprecate('symfony/framework-bundle','6.2','The alias "%alias_id% is deprecated. Use "FooBarAlias" instead.')

carsonbot added Status: Needs Review Feature FrameworkBundle labels

Jun 30, 2022

carsonbot added this to the6.2 milestone

Jun 30, 2022

94noni force-pushed thedebug-container-deprecated branch fromf68d488 to4afa6c5Compare

June 30, 2022 13:38

This comment was marked as outdated.

Copy link

Contributor

ogizanagi commentedJul 3, 2022

What about showing the deprecation message as well?
Regarding the txt output, this could even be shown in a warning block.

Copy link

ContributorAuthor

94noni commentedJul 3, 2022

@ogizanagi it could indeed be a nice idea!

But as there are lot of test files to update, I would prefer asking first a consensus before doing the proposed change if you dont mind :)

GromNaN reviewed

Jul 3, 2022

View reviewed changes

Copy link

Member

GromNaN left a comment

Choose a reason for hiding this comment

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

That's a very useful information. I like the idea of colored warning in case of depreciation.

src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/builder_priority_tag.xmlShow resolvedHide resolved

Copy link

ContributorAuthor

94noni commentedJul 4, 2022

@GromNaN @ogizanagi oki, I will try adding the deprecated message, and a colored output for when applicable

94noni force-pushed thedebug-container-deprecated branch 4 times, most recently fromf4ac1a4 to32562a8Compare

July 4, 2022 08:18

Copy link

ContributorAuthor

94noni commentedJul 4, 2022

With new code changes, I can get such results (with faking a service deprecation)

Copy link

Contributor

ogizanagi commentedJul 4, 2022•
edited
Loading

@94noni : Sorry to make you change this again, I should have been clearer:
what about removing the info from the table output to show the deprecation message if the service is deprecated, using$io->warning() at the end of the command output?

Copy link

ContributorAuthor

94noni commentedJul 4, 2022

@ogizanagi hey, do you mean like in my screenshot and written above the ! [Note] but as warning?

if yes I can rework as requested :)

Copy link

Contributor

ogizanagi commentedJul 4, 2022

@94noni : exactly what I meant :)

94noni force-pushed thedebug-container-deprecated branch 2 times, most recently from8e2d3cd to6a30629Compare

July 5, 2022 08:43

Copy link

ContributorAuthor

94noni commentedJul 5, 2022

@ogizanagi @GromNaN PR reworked and description updated based on addressed comments :)

Copy link

Member

chalasr commentedJul 5, 2022

Looks good to me. Can you please go ahead with the missing test(s)?

Copy link

ContributorAuthor

94noni commentedJul 5, 2022

@chalasr The only failed test I can see in the above checks is about1) Symfony\Component\VarDumper\Tests\Server\ConnectionTest::testDump seem unrelated isn't it?

stof requested changes

Jul 5, 2022

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.phpShow resolvedHide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels

Jul 5, 2022

Copy link

Member

chalasr commentedJul 5, 2022

What I mean is that this feature should be covered by a test if possible

94noni commented

Jul 6, 2022

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ContainerDebug/config.yml

		deprecated:
		class:Symfony\Bundle\FrameworkBundle\Tests\Fixtures\DeprecatedClass
		deprecated:The "%service_id%" service is deprecated since foo/bar 1.9 and will be removed in 2.0. Use "public" service instead.
		deprecated_alias:

Copy link

ContributorAuthor

94noniJul 6, 2022

Choose a reason for hiding this comment

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

cfdoc

94noni commented

Jul 6, 2022

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ContainerDebug/config.yml

		private_alias:
		alias:public
		public:false
		deprecated:

Copy link

ContributorAuthor

94noniJul 6, 2022•
edited
Loading

Choose a reason for hiding this comment

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

cfdoc +PR

94noni force-pushed thedebug-container-deprecated branch fromcf6e55d toe9a5d5cCompare

July 6, 2022 15:21

94noni mentioned this pull request

Jul 8, 2022

Fix doc service deprecationsymfony/symfony-docs#16952

Merged

94noni force-pushed thedebug-container-deprecated branch 6 times, most recently from4bb7984 to0f69408Compare

July 8, 2022 08:00

Copy link

ContributorAuthor

94noni commentedJul 8, 2022

status: needs review

carsonbot added Status: Needs Review and removed Status: Needs Work labels

Jul 8, 2022

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request

Jul 8, 2022

minor#16952Fix doc service deprecation (94noni)

4fa18aa

This PR was submitted for the 6.0 branch but it was merged into the 5.4 branch instead.Discussion----------Fix doc service deprecationHi,Based on [this PR](symfony/symfony#46814), I've followed [the doc](https://symfony.com/doc/current/service_container/alias_private.html#deprecating-services)But when tests executed, I got [this error](symfony/symfony#46814 (comment))So I propose to fix the doc accordingly hereThank youCommits-------ffdb7e6 Fix documentation on declaration of service deprecation in YAML format

94noni mentioned this pull request

Jul 13, 2022

[FrameworkBundle] Add "Usages" to debug:container output#38903

Merged

Copy link

ContributorAuthor

94noni commentedJul 18, 2022

With#38903 being recently merged there are lot of conflicts
May I ask from reviewers another round of reviews on the core code, before taking time to fix all tests conflicts?
thanks :)

ogizanagi reviewed

Jul 18, 2022

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md OutdatedShow resolvedHide resolved

src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php OutdatedShow resolvedHide resolved

src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php OutdatedShow resolvedHide resolved

src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/Descriptor.php OutdatedShow resolvedHide resolved

fabpot approved these changes

Jul 20, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jul 20, 2022

Copy link

Member

fabpot commentedJul 20, 2022

@94noni Can you rebase this PR to get rid of the merge commit (that prevents me from merging this PR)?

Copy link

ContributorAuthor

94noni commentedJul 20, 2022

@fabpot I'll handle this very soon and ping back

94noni force-pushed thedebug-container-deprecated branch 2 times, most recently fromeb6b62e to5a93272Compare

July 21, 2022 06:58

94noni changed the title~~[FrameworkBundle] Add service deprecation on debug:container command output~~[FrameworkBundle] Add service and alias deprecation message to debug:container <name> output

Jul 21, 2022