add support for PHP 8 #24
Conversation
| @@ -286,8 +287,12 @@ public function testRun(): void | |||
|
|
|||
| private function makeLineStream(array $lines) | |||
| { | |||
| $fp = fopen('data://text/plain;base64,' . base64_encode(implode("\n", $lines)), 'ab+'); | |||
|
|
|||
There was a problem hiding this comment.
I don't think this code is problematic for the input stream and it works well.
But for the output stream is not working, as data wrapper is not writable.
And maybe that's helpful for the future, as the STDIN input is similar, not writable and we can generate a proper stream that will issue a warning in case app tries to write to input.
I would suggest to:
- keep the function as it was but rename it to
makeInputStream()and keep use it for$inputStream. - create a new function
makeOutputStream()that just fopen tophp://memoryand use it for$outputStream.
composer.json
Outdated
There was a problem hiding this comment.
Don't you think that we should remove the platform from config (line 19), so we won't have any hard restriction on version number?
There was a problem hiding this comment.
I think we might want to remove that "php": "7.4" platform config.
If we don't run composer as a container (that runs on various php version, usually latest), it's not needed anymore.
But I'm not sure if it bothers us if we keep it at minimum. So I think we don't need to change it now.
|
Looks good. |
The update to PHP 8.* would probably greatly improve performance as JIT is ideal for these kind of long running PHP processes.