Skip to content

[Messenger] add support for abstract handlers #31429

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 4, 2020
Merged

[Messenger] add support for abstract handlers #31429

merged 1 commit into from
Feb 4, 2020

Conversation

timiTao
Copy link
Contributor

@timiTao timiTao commented May 8, 2019 •
edited
Loading

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #31417
License MIT
Doc PR symfony/symfony-docs#...

Added handling abstract handler.

@timiTao timiTao changed the title add abstract handler [Messenger] add abstract handler May 8, 2019
@timiTao timiTao force-pushed the abstract-handler branch from d511c10 to 5ab44c7 Compare May 8, 2019 21:16
@nicolas-grekas nicolas-grekas added this to the next milestone May 9, 2019
nicolas-grekas
nicolas-grekas reviewed May 9, 2019
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I'd be OK to add this for 4.3 - here are some comments before.

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 May 9, 2019
@nicolas-grekas nicolas-grekas changed the base branch from master to 4.3 May 9, 2019 07:59
@timiTao timiTao force-pushed the abstract-handler branch from 5ab44c7 to a42682c Compare May 9, 2019 21:57
@nicolas-grekas
Copy link
Member

Can you please rebase -i to get rid of the extra commits?

@timiTao timiTao force-pushed the abstract-handler branch from a42682c to 81726f1 Compare May 11, 2019 19:14
@sroze
Copy link
Contributor

sroze commented May 12, 2019

@timiTao can you rebase to get rid of these extra commits in your branch please?

@timiTao timiTao mentioned this pull request May 13, 2019
Closed
@timiTao timiTao force-pushed the abstract-handler branch 2 times, most recently from 800189c to 17d5a2d Compare May 13, 2019 06:45
juanmiguelbesada
juanmiguelbesada reviewed May 15, 2019
View reviewed changes
Copy link
Contributor

@juanmiguelbesada juanmiguelbesada left a comment

Choose a reason for hiding this comment

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

In my opinion, I think is better set $container as first parameter of getDefinitionClass method (to keep consistent with other methods)

@timiTao timiTao force-pushed the abstract-handler branch from b9b858e to c9a3f53 Compare May 19, 2019 20:02
@fabpot fabpot modified the milestones: 4.3, next May 27, 2019
@timiTao timiTao force-pushed the abstract-handler branch from c9a3f53 to 3da0f81 Compare May 30, 2019 06:07
@nicolas-grekas nicolas-grekas changed the title [Messenger] add abstract handler [Messenger] fix support for abstract handlers Jul 30, 2019
@nicolas-grekas nicolas-grekas changed the title [Messenger] fix support for abstract handlers [Messenger] add support for abstract handlers Jul 30, 2019
@nicolas-grekas nicolas-grekas changed the base branch from 4.3 to 4.4 July 30, 2019 19:14
@nicolas-grekas nicolas-grekas changed the base branch from 4.4 to master February 4, 2020 13:57
@nicolas-grekas
Copy link
Member

Thank you @timiTao.

nicolas-grekas added a commit that referenced this pull request Feb 4, 2020
@nicolas-grekas
feature #31429 [Messenger] add support for abstract handlers (timiTao)
2aca43f 
This PR was submitted for the 4.4 branch but it was merged into the 5.1-dev branch instead.

Discussion
----------

[Messenger] add support for abstract handlers

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | no
| New feature?  | yes <!-- don't forget to update src/**/CHANGELOG.md files -->
| 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 | #31417    <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Added handling abstract handler.

Commits
-------

22e59f3 [Messenger] fix support for abstract handlers
@nicolas-grekas nicolas-grekas merged commit 22e59f3 into symfony:master Feb 4, 2020
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juanmiguelbesada juanmiguelbesada juanmiguelbesada left review comments

@sroze sroze sroze requested changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@dunglas dunglas Awaiting requested review from dunglas

@lyrixx lyrixx Awaiting requested review from lyrixx

@xabbuh xabbuh Awaiting requested review from xabbuh

Assignees
No one assigned
Labels
Feature Status: Reviewed
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
6 participants
@timiTao @nicolas-grekas @sroze @juanmiguelbesada @fabpot @carsonbot