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

[Messenger] Don't deep-merge senders configuration #32970

Open
wants to merge 1 commit into
base: 4.4
from

Conversation

@chalasr
Copy link
Member

commented Aug 5, 2019

Q A
Branch? 4.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #32953
License MIT
Doc PR -
@fabpot
fabpot approved these changes Aug 6, 2019
Copy link
Member

left a comment

Not sure we can do it in 4.3.

@sroze
sroze approved these changes Aug 18, 2019
Copy link
Member

left a comment

Makes sense for the Messenger keys indeed. But shouldn't we do this in 4.4 only because it's still clearly a BC break?

@fabpot

This comment has been minimized.

Copy link
Member

commented Aug 18, 2019

I agree that this should be for 4.4.

@nicolas-grekas
Copy link
Member

left a comment

just putting a "needs work" here, see my last inline comment :)

@chalasr chalasr force-pushed the chalasr:no-deep-merge branch from 53ea26e to cbe197f Sep 13, 2019

@chalasr chalasr requested review from dunglas, lyrixx and xabbuh as code owners Sep 13, 2019

@chalasr chalasr changed the base branch from 4.3 to 4.4 Sep 13, 2019

@chalasr chalasr removed request for dunglas, lyrixx and xabbuh Sep 13, 2019

@chalasr chalasr changed the title [Messenger][HttpClient] Don't deep-merge senders/transports/clients configuration [Messenger] Don't deep-merge senders configuration Sep 13, 2019

@chalasr chalasr force-pushed the chalasr:no-deep-merge branch from cbe197f to 976ebdf Sep 13, 2019

@chalasr chalasr force-pushed the chalasr:no-deep-merge branch from 976ebdf to 20d19ac Sep 13, 2019

@chalasr

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2019

PR updated to only disable deep merging for the framework.messenger.routing.senders prototype and fix the related ticket. Also rebased on 4.4 with CHANGELOG entry.

As @nicolas-grekas suggests, we need to discuss other parts of the config tree case by case, let's move forward on this one.

@chalasr chalasr modified the milestones: 4.3, next Sep 13, 2019

@lyrixx
lyrixx approved these changes Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.