search

thephpleague / tactician-bundle

Bundle to integrate Tactician with Symfony projects
MIT License
245 stars 43 forks source link

Symfony 5 support #119

Closed webinarium closed 4 years ago

webinarium commented 4 years ago

This PR fixed #118.

rosstuck commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

(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 commented 4 years ago

(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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 :)