10000 [Messenger] Do not require doctrine/messenger · Issue #36740 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger] Do not require doctrine/messenger #36740

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
Closed
tyx opened this issue May 7, 2020 · 6 comments · Fixed by #36785
Closed

[Messenger] Do not require doctrine/messenger #36740

tyx opened this issue May 7, 2020 · 6 comments · Fixed by #36785
Labels
Bug Messenger Status: Needs Review

Comments

@tyx
Copy link
Contributor
tyx commented May 7, 2020

Symfony version(s) affected: 5.1.0@beta

Description

Since #35422 we split transport in dedicated package. Nice addition ! But we require doctrine/messenger by default ... and this one will install doctrine dbal.

I have not dbal installed for now and here are the new package installed:
Screenshot from 2020-05-07 14-31-22

I guess we made this change to keep BC. But it is very annoying. Before this change I had only doctrine messenger part, now I have all doctrine stuff.

How to reproduce

Install symfony/messenger without doctrine pack.

I should admit I have no idea how fix this issue.
@tyx tyx added the Bug label May 7, 2020
@shouze
Copy link
shouze commented May 7, 2020

@tyx this is because prior to #35422 symfony/messenger was only requiring doctrine/dbal as you can see.

Since that it requires symfony/doctrine-messenger that requires a lot more of doctrine dependencies. @Nyholm don't we have a better option regarding dependencies splitting? 🤔

@tyx
Copy link
Contributor Author
tyx commented May 8, 2020

dbal was a dev requirement before ! As doctrine transport was embedded but not mandatory.

The problem really came when we splitted transport in different package and add doctrine one as requirement of messenger.

@fabpot
Copy link
Member
fabpot commented May 8, 2020

/cc @Nyholm

@Nyholm
Copy link
Member
Nyholm commented May 8, 2020

Thank you for this issue. That is something I did miss before.

To make it work as it did in 5.0, I think we should move the doctrine/persistence and doctrine/dbal to the require-dev section. This will treat symfony/doctrine-messenger as a special package until 6.0. But that is fine.

@tyx
Copy link
Contributor Author
tyx commented May 12, 2020

Hi @Nyholm can I make the PR to help ? Or do you prefer make it on your own ?

@Nyholm
Copy link
Member
Nyholm commented May 12, 2020

No no no. Please make a PR :)

@tyx tyx mentioned this issue May 12, 2020
Merged
nicolas-grekas added a commit that referenced this issue May 16, 2020
@nicolas-grekas
minor #36785 [Messenger] Move doctrine deps to require-dev (tyx)
02cedc3 
This PR was merged into the 5.1-dev branch.

Discussion
----------

[Messenger] Move doctrine deps to require-dev

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #36740
| License       | MIT

To avoid requiring all doctrine stuff when require symfony/messenger
(that require symfony/doctrine-messenger to ensure BC)

Commits
-------

b73b26e Move doctrine deps to require-dev
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Messenger Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Messenger] Move doctrine deps to require-dev tyx/symfony
7 participants
@fabpot @shouze @nicolas-grekas @tyx @Nyholm @xabbuh @carsonbot
0