theofidry / PsyshBundle

A command line REPL bundle for Symfony using PsySH.
MIT License
208 stars 29 forks source link

Feature/symfony6 #65

Closed Chris53897 closed 2 years ago

Chris53897 commented 3 years ago

Hi. I started to work on a solution for symfony6 support. I fixed deprecations and find solutions to support Symfony 4.4 and 5.4

The main-problem is missing return type in psysh/Shell.php. Since they still support php 5.6 as lowest version, i think they do not want to change this. https://github.com/bobthecow/psysh/blob/main/src/Shell.php#L304

PHP Fatal error: Declaration of Psy\Shell::run(?Symfony\Component\Console\Input\InputInterface $input = null, ?Symfony\Component\Console\Output\OutputInterface $output = null) must be compatible with Symfony\Component\Console\Application::run(?Symfony\Component\Console\Input\InputInterface $input = null, ?Symfony\Component\Console\Output\OutputInterface $output = null): int in /home/runner/work/PsyshBundle/PsyshBundle/vendor/psy/psysh/src/Shell.php on line 304

See testrun fails (do not trust the green arrow).

There are 2 possible outcomes. a) Psysh removes Symfony 6 support. => This PR should remove the support the Symfony 6 support in composer.json b) Psysh supports Symfony 6 and bump php-version. => In this case i will wait

Chris53897 commented 2 years ago

The new release of psych is supporting symfony 6 now.

msheakoski commented 2 years ago

Hey @Chris53897! Thank you for making this PR. Because it is doing many different things, it might be more difficult for @theofidry to merge it in. The addition of the GitHub workflows and upgrade to PHPUnit 9 would be great standalone PRs.

I sent a PR over to you at Chris53897/PsyshBundle/pull/1 with the minimum set of changes to get Symfony 6 support added. PsyshBundle is one of the last packages in my projects holding composer back from allowing an upgrade, so hopefully this does the trick!

Chris53897 commented 2 years ago

@msheakoski You are right. This PR does to much things. Github Actions were needed to test is for me. PHPUnit Changes are definitely another PR.

theofidry commented 2 years ago

Thank you @Chris53897 @msheakoski

msheakoski commented 2 years ago

@theofidry I missed a few things with whitespace and one of the config files. Should I just PR this directly on your repo since you already merged it?

https://github.com/Chris53897/PsyshBundle/pull/2/files

theofidry commented 2 years ago

@msheakoski sure

msheakoski commented 2 years ago

@theofidry It looks like you made some changes since yesterday that supersede the items that I missed, so I think we're all set. I will give your new release a try with Symfony 6. Thanks and happy new year!