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

Symfony 5 support #119

Merged
merged 7 commits into from Dec 31, 2019
Merged

Symfony 5 support #119

merged 7 commits into from Dec 31, 2019

Conversation

webinarium
Copy link
Contributor

This PR fixed #118.

@rosstuck
Copy link
Member

rosstuck commented Dec 9, 2019

Hey, thanks for getting a start on this!

Getting a passing build on (my somewhat outdated) test suite is going to take a bit more work. I've started here and have green tests but I need to untangle the travis versions as well ( https://github.com/thephpleague/tactician-bundle/compare/symfony-5?expand=1 )

If you'd like to give that a go, that would be the major blocker. Honestly, I may just tag a new v1.2 and start dropping older version support. Symfony itself no longer supports 2.8, for example. Alternately, just roll forward with getting #115 done would be ideal but sounds like there's still a good chunk to do.

@webinarium
Copy link
Contributor Author

The PR is really all about to append |^5.0 to the composer.json, so simply tagging it with 1.2 would work. But I understand if you prefer to do the things right, and I'm fine to wait until you untangled old travis builds and completed #115. No worries, there is no rush here. :)

Thank you for the tactician, btw! I've used it for years to apply CQRS to my Symfony projects, but never have said "thanks" before.

@nrutman
Copy link

nrutman commented Dec 30, 2019

Just out of curiosity, is there any update on this issue/PR? I'd love to use this bundle on a Symfony 5 project, but currently cannot do so.

@nrutman
Copy link

nrutman commented Dec 30, 2019

(If this is going to take a while, I suppose I can simply fork this PR and use it until a new release is cut. Just let me know what you advise.)

@webinarium
Copy link
Contributor Author

(If this is going to take a while, I suppose I can simply fork this PR and use it until a new release is cut. Just let me know what you advise.)

Another option, which worked for me perfectly - use the new 'Messenger' Symfony component. :) I switched to this component and it works as a charm, with very few changes in existing sources (actually, just had to replace handle with __invoke in all my handlers and that's it).

Check it out, particularly Multiple Buses.

@rosstuck
Copy link
Member

I haven't had time to work on this over the holidays. As mentioned above, there's a bit more work to do here to get a passing build on this.

@webinarium If you've switched over to Symfony Messenger, are you planning to take this any further?

@webinarium
Copy link
Contributor Author

If you've switched over to Symfony Messenger, are you planning to take this any further?

Sorry, didn't know you're expecting something from me regarding the PR. As far as I can see, failed builds were broken before the PR, and I thought you are on it.

Anyway, I upmerged my PR with your recent commits and this fixed most of the failures - only 2.8 builds failed on composer stage with the "Allowed memory size of 1610612736 bytes exhausted" message. So, I've increased the memory limit from 1.5G to 2G in Travis - now everything passes! :)

Is there anything else I can help you with regarding the PR? It looks good to me now.

@rosstuck
Copy link
Member

My apologies for bad communication then. :) I agree looks good now and the raised limit is okay with me, I'll trim down the Travis Matrix and tag a new release this evening or so. I appreciate you putting in the extra work here, think you've made a lot of folks very happy :)

@rosstuck rosstuck merged commit 555850c into thephpleague:master Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Symfony 5
3 participants