Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix framework configuration when messenger uses without console #43202

Merged
merged 1 commit into from
Sep 29, 2021

Conversation

upyx
Copy link
Contributor

@upyx upyx commented Sep 27, 2021

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? yes
Tickets Fix #43133 (review)
License MIT
Doc PR later

We hurried, an addition to #43133

It fixes #43133 (review)
It adds the forgotten CHANGELOG & UPGRADE

I'm not sure about the CHANGELOG format. I've done as people do but... it says "New entries must be added on top of the list." However, nobody does it 🤷

I don't know how to test the fix, because of class_exists() call that cannot be easily mocked.

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php:245:

if (class_exists(Application::class)) {
    $loader->load('console.php');
    /* ... */
}

@upyx upyx requested a review from sroze as a code owner September 27, 2021 17:42
@carsonbot carsonbot added this to the 5.4 milestone Sep 27, 2021
@upyx upyx changed the title Fix reset on message Fix framework configuration when messenger uses without console Sep 27, 2021
@fabpot
Copy link
Member

fabpot commented Sep 27, 2021

can you fix fabbot issue?

@upyx
Copy link
Contributor Author

upyx commented Sep 27, 2021

I've added a test for messenger without console. I wrapped class_exists() call to protected hasConsole(), and mock it in tests.
I've touched pretty old code, and we should be careful before merge it.

@upyx
Copy link
Contributor Author

upyx commented Sep 28, 2021

@lyrixx 👋 I'd love to finish the work which we started :) Could you check it, please?

Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

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

👍🏼

@fabpot
Copy link
Member

fabpot commented Sep 29, 2021

Thank you @upyx.

@fabpot fabpot merged commit 7244d83 into symfony:5.4 Sep 29, 2021
fabpot added a commit that referenced this pull request Nov 8, 2021
…nfig option default to true (upyx)

This PR was merged into the 6.0 branch.

Discussion
----------

[FrameworkBundle] Make the messenger.reset_on_message config option default to true

| Q             | A
| ------------- | ---
| Branch?       | 6.0
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | no
| License       | MIT
| Doc PR        | no

It changes the "framework.messenger.reset_on_message" configuration option to true and changes deprecation warnings.

~It's waiting fixes from #43202~

Commits
-------

e3eecd1 [FrameworkBundle] Make the messenger.reset_on_message config option default to true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants