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

Changing messenger bus id from 'message_bus' to 'messenger.default_bus' #30690

Merged
merged 1 commit into from Apr 1, 2019

Conversation

Projects
None yet
7 participants
@ktherage
Copy link

commented Mar 25, 2019

Changing messenger bus tag from 'message_bus' to 'messenger.message_bus'

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? Maybe
Deprecations? no
Tests pass? yes
Fixed tickets #30670
License MIT
Doc PR

All is in the title.
This PR change the tag of the default bus from 'message_bus' to 'messenger.message_bus'.

@weaverryan

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

We don't need it due to the experimental, but do we want a BC layer here?

@weaverryan weaverryan changed the title Changing messenger bus tag from 'message_bus' to 'messenger.message_bus' Changing messenger bus id from 'message_bus' to 'messenger.message_bus' Mar 26, 2019

@chalasr

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

👍 for adding BC layer

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

We don't need a BC layer, but we definitely need some documentation in the CHANGELOG to help people migrate their code.

@sroze

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

Why not messenger.default_bus? So it's explicit that it's the default one.

We already have:

  • translator.default
  • router.default
  • cache.default_*
@weaverryan

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Personally, I do like messenger.default_bus

@sroze

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

I'd add a BC layer as well, especially given the fact we have all these one-word services in Symfony (router, translation, etc...)

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

The bc layer is a one liner: a deprecated alias should do it.

@sroze

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

@ktherage can you do these two changes (BC layer + messenger.default_bus)?

@ktherage ktherage force-pushed the ktherage:messenger-default-bus-name branch from dc50758 to 7e2d52c Apr 1, 2019

ktherage pushed a commit to ktherage/symfony that referenced this pull request Apr 1, 2019

@ktherage

This comment has been minimized.

Copy link
Author

commented Apr 1, 2019

@sroze i made the wanted changes but i'm not really sure that changes about the BC layer are those you wanted. Feel free to tell me what's wrong (on Slack i am more reactive because i get noticed as soon as i got a message).

@chalasr chalasr changed the title Changing messenger bus id from 'message_bus' to 'messenger.message_bus' Changing messenger bus id from 'message_bus' to 'messenger.default_bus' Apr 1, 2019

@ktherage ktherage force-pushed the ktherage:messenger-default-bus-name branch from 7e2d52c to 0f69c18 Apr 1, 2019

ktherage pushed a commit to ktherage/symfony that referenced this pull request Apr 1, 2019

@ktherage ktherage force-pushed the ktherage:messenger-default-bus-name branch from 0f69c18 to 16f5711 Apr 1, 2019

ktherage pushed a commit to ktherage/symfony that referenced this pull request Apr 1, 2019

@weaverryan

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

@ktherage check the tests - one is failing now :)

@ktherage

This comment has been minimized.

Copy link
Author

commented Apr 1, 2019

@weaverryan yes sorry 😅. i'll fix it.

@ktherage ktherage force-pushed the ktherage:messenger-default-bus-name branch from 16f5711 to 3cee1ca Apr 1, 2019

@chalasr

chalasr approved these changes Apr 1, 2019

Copy link
Member

left a comment

Remaining failures are unrelated (outdated base branch)

@fabpot

fabpot approved these changes Apr 1, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

Thank you @ktherage.

@fabpot fabpot merged commit 3cee1ca into symfony:master Apr 1, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Apr 1, 2019

feature #30690 Changing messenger bus id from 'message_bus' to 'messe…
…nger.default_bus' (THERAGE Kévin)

This PR was merged into the 4.3-dev branch.

Discussion
----------

Changing messenger bus id from 'message_bus' to 'messenger.default_bus'

Changing messenger bus tag from 'message_bus' to 'messenger.message_bus'

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | Maybe
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30670
| License       | MIT
| Doc PR |

All is in the title.
This PR change the tag of the default bus from 'message_bus' to 'messenger.message_bus'.

Commits
-------

3cee1ca #30690 - Changing messenger bus id from 'message_bus' to 'messenger.default_bus'

@ktherage ktherage deleted the ktherage:messenger-default-bus-name branch Apr 1, 2019

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

nicolas-grekas added a commit that referenced this pull request May 5, 2019

bug #31384 [Mailer][Messenger] Changing messenger bus id from 'messag…
…e_bus' to 'messenger.default_bus' (Koc)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Mailer][Messenger] Changing messenger bus id from 'message_bus' to 'messenger.default_bus'

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #30690
| License       | MIT
| Doc PR        | -

Follow up of #30690

Commits
-------

d3079e0 Changing messenger bus id from 'message_bus' to 'messenger.default_bus'

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

ktherage pushed a commit to ktherage/symfony that referenced this pull request May 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.