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

Fixes #917: Allow Robo to be used with Symfony 5. #940

Merged
merged 1 commit into from
May 27, 2020
Merged

Conversation

greg-1-anderson
Copy link
Member

Overview

This pull request:

  • Fixes a bug
  • Adds a feature
  • Breaks backwards compatibility
  • Has tests that cover changes
  • Adds or fixes documentation

Summary

Allow Symfony 5.

@greg-1-anderson
Copy link
Member Author

There's a test failure here from EventDispatcher::dispatch() that needs an API update.

We also have a problem with Symfony/Process requiring a factory method instead of a constructor to use a string parameter to exec. We might need to test which version of Symfony/Process we have, as the new factory method is not available in older versions of Symfony/Process (and we'd need to do a major version bump to drop old Symfony/Process versions).

@alsar
Copy link

alsar commented Apr 13, 2020

The event dispatcher in v5 changed the parameter signature from string to object. So there needs to be a check (via reflection) which type the function accepts.

For the Process component a simple method_exists check should suffice

@alsar
Copy link

alsar commented Apr 13, 2020

The method fromShellCommandline is already available since version 4.2 so there should be no problem to change new Process($this->getCommand(), getcwd()) to Process::fromShellCommandline($this->getCommand(), getcwd()) because Robo requires symfony/process ^4.4.3|^5.

@greg-1-anderson
Copy link
Member Author

I forgot that I already dropped support for older versions of Symfony/Process in Robo 2. That turns out to have been foresightful; makes things easier here.

@c33s
Copy link
Contributor

c33s commented Apr 14, 2020

i won't drop support for a symfony version which is still supported. https://symfony.com/releases/3.4 End of bug fixes: November 2020 End of security fixes: November 2021

a task runner should be compatible with legacy php and symfony versions as long as possible. bc layers would be awesome.

just my opinion

@greg-1-anderson
Copy link
Member Author

Yeah, with more maintainers maybe it wouldn't be a big deal to include more Symfony versions. There's still the Robo 1.x branch for Symfony 3. If you need Symfony 3 AND Symfony 5, you could consider back-porting this PR to the 1.x branch.

Pretty soon we have to think about Composer 2 as well, though; limiting supported versions greatly reduces the testing permutations.

@greg-1-anderson
Copy link
Member Author

For those who wish to support a wide variety of Symfony versions, "consolidation/robo": "^1 | ^2" should work well in many instances. The Robo 1.x and 2.x APIs are pretty similar.

@greg-1-anderson greg-1-anderson merged commit 1d285d7 into master May 27, 2020
@greg-1-anderson
Copy link
Member Author

This fixed many incompatibilities between Robo and Symfony 5, but not all.

@greg-1-anderson greg-1-anderson deleted the symfony-5 branch May 27, 2020 22:20
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.

3 participants