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

[Console][FrameworkBundle] Fix missingprofile option for console commands#52434

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

Merged
chalasr merged 1 commit intosymfony:6.4fromkeulinho:patch-1
Nov 7, 2023

Conversation

keulinho
Copy link
Contributor

@keulinhokeulinho commentedNov 3, 2023
edited by OskarStark
Loading

QA
Branch?6.4
Bug fix?yes
New feature?no
Deprecations?no
IssuesFix#52433
LicenseMIT

@keulinho
Copy link
ContributorAuthor

Unit test failure seems unrelated to this change 🤔

@keulinho
Copy link
ContributorAuthor

Also the fail in the appveyor CI seems unrelated as it is some error in the redis integration test for the messenger component 🤔

@OskarStark
Copy link
Contributor

friendly ping@HeahDude as you contributed this feature, thanks

@symfonysymfony deleted a comment fromcarsonbotNov 3, 2023
@GromNaN
Copy link
Member

I could not reproduce the issue#52433.

This listener is part of theFrameworkBundle, and so it should be used withSymfony\Bundle\FrameworkBundle\Console \Application. Could you provide more details on your application or test setup?

@keulinho
Copy link
ContributorAuthor

keulinho commentedNov 3, 2023
edited by OskarStark
Loading

Ok took a look again and seemed i messed something up with the command tester, that seems to work as it does not dispatch the event.

However manually triggering events by dispatching theConsoleCommandEvent does not work anymore.
E.g.

$commandEvent =newConsoleCommandEvent($command,$input,$output);$kernel->getContainer()->get('event_dispatcher')->dispatch($commandEvent, ConsoleEvents::COMMAND);

We used something like that in an internal command tester, that's why i first assumed the issue is in the symfony command tester as well.

However, I still think the fix makes sense, as you should not rely in the listener that the event was correctly configured in the application class. IMHO the benefit of using events is to decouple different parts of the system from each other, but this assumption (that there is always anprofile option) in the listener basically couples the listener to the application class-

@chalasr
Copy link
Member

I've no strong opinion here. I think I can live with that coupling, especially given it's part of a bundle

Copy link
Contributor

@HeahDudeHeahDude left a comment

Choose a reason for hiding this comment

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

Given#52433 (comment), I think this fix looks sensible. Thanks!

@chalasr
Copy link
Member

Thank you@keulinho.

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

@chalasrchalasrchalasr approved these changes

@HeahDudeHeahDudeHeahDude approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.

[Console][FrameworkBundle]--profile option might not be set on every command
6 participants
@keulinho@OskarStark@GromNaN@chalasr@HeahDude@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp