Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Migrate the Security Bundle console commands to use the new invokable feature#62573

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

Conversation

@Kefisu
Copy link
Contributor

QA
Branch?8.1
Bug fix?no
New feature?yes
Deprecations?no
IssuesNone
LicenseMIT

This PR migrates the Console Commands present in the Security Bundle to use the new invokable command feature introduced in Symfony 7.3.

This PR is targeting the Symfony 8.1 branch as it is removing theextends Command syntax and public API.

RemcoSmitsDev reacted with rocket emoji
@carsonbotcarsonbot added this to the8.1 milestoneNov 29, 2025
@carsonbotcarsonbot changed the titleMigrate the SecurityBundle console commands to use the new invokable feature Migrate the SecurityBundle console commands to use the new invokable featureNov 29, 2025
@KefisuKefisu changed the title Migrate the SecurityBundle console commands to use the new invokable feature [SecurityBundle] Migrate the SecurityBundle console commands to use the new invokable featureNov 29, 2025
@KefisuKefisu changed the title [SecurityBundle] Migrate the SecurityBundle console commands to use the new invokable feature [SecurityBundle] Migrate the console commands to use the new invokable featureNov 29, 2025
@xabbuh
Copy link
Member

I am not sure that we really want to do that as it calls for conflicts when merging7.4 into8.1.

yceruto reacted with thumbs up emoji

@KefisuKefisuforce-pushed thefeature/migrate-sec-bundle-commands-to-invoked-commands branch from707df92 tof663eb8CompareNovember 29, 2025 13:31
---

* Add support for the`clientHints`,`prefetchCache`, and`prerenderCache``ClearSite-Data` directives
* Migrate the`Symfony/Bundle/SecurityBundle/Command/DebugFirewallCommand` and`Symfony/Bundle/SecurityBundle/Command/SecurityRoleHierarchyDumpCommand` to the invokable console commands feature
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it needs to be documented in the CHANGELOG as there are no consequences for the consumers of these commands.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Alright, the line has been removed

@carsonbotcarsonbot changed the title [SecurityBundle] Migrate the console commands to use the new invokable feature Migrate the console commands to use the new invokable featureNov 29, 2025
@KefisuKefisu changed the title Migrate the console commands to use the new invokable feature Migrate the Security Bundle console commands to use the new invokable featureNov 30, 2025
@wouterj
Copy link
Member

wouterj commentedNov 30, 2025
edited
Loading

Thanks for proposing this "modernization" :)

However, I don't think we can do this given ourbackwards compatibility promise unfortunately. Given these classes aren't internal, we must make sure they remain ancestors of theCommand class as applications might rely on this (e.g. when passing the instance to a method expecting aCommand type). The same applies to theconfigure()/execute() commands, someone might have decorated or overridden these methods in a child class and we must make sure that doesn't break.

So I don't think we can do anything here, and all existing commands in Symfony must keep using theCommand public API instead of the invokable command one (unless someone comes up with a genius smooth upgrade path).

derrabus reacted with thumbs up emoji

@derrabus
Copy link
Member

I agree with@wouterj. It's a BC break which we don't gain much from. I don't believe we should do this.

@nicolas-grekas
Copy link
Member

Thanks for proposing though.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot requested changes

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

8.1

Development

Successfully merging this pull request may close these issues.

7 participants

@Kefisu@xabbuh@wouterj@derrabus@nicolas-grekas@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp