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.

Sign up for GitHub

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

[RoadRunner] Add support for RoadRunner 2023 and 2024 #172

Merged
merged 6 commits into from
Nov 17, 2024
Merged

[RoadRunner] Add support for RoadRunner 2023 and 2024 #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f86a9a6
Add support for RoadRunner 2024
msmakouz May 20, 2024
c2afca9
Add php-version option to Psalm
msmakouz May 20, 2024
a0d0a5e
Remove spiral/roadrunner dependency
msmakouz Jun 18, 2024
67da953
Add symfony/console 5.4
msmakouz Jun 18, 2024
4898517
Change spiral/roadrunner-http versions
msmakouz Jun 18, 2024
17ee752
Add spiral/roadrunner-worker
msmakouz Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/static.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ jobs:
composer-options: "--ignore-platform-reqs" # required for PHP 8.2 as not yet official supported

- name: Psalm
run: psalm --no-progress --output-format=github
run: psalm --php-version=8.2 --no-progress --output-format=github
Copy link
Member

@alexander-schranz alexander-schranz Jun 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unfamiliar with pslam @Nyholm can you have a look at this change if you are fine with it or not?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is inferred from the PHP version we are running on.

I dont mind adding it, but it would be nice to hear what value it brings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msmakouz can you answer that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexander-schranz @Nyholm In the Psalm action, PHP 8.2 is used, and the latest dependencies are used. However, without this option, Psalm checks with the PHP version specified as the minimum in composer.json, and it displays two errors:

Error: src/roadrunner-nyholm/src/Runner.php:23:19: UndefinedClass: Class, interface or enum named Spiral\RoadRunner\Worker does not exist (see https://psalm.dev/019)
Error: src/roadrunner-symfony-nyholm/src/Runner.php:35:19: UndefinedClass: Class, interface or enum named Spiral\RoadRunner\Worker does not exist (see https://psalm.dev/019)

https://github.com/php-runtime/runtime/actions/runs/9156994654/job/25172484593

I assume this is because the package spiral/roadrunner-worker (v3.6.0) requires a minimum PHP version of 8.1. If this option is added, these errors are not present. Need to specify this option or lower the PHP version in the action to 8.0 so that the dependencies requiring a minimum PHP version of 8.0 are installed.

Copy link
Member

@alexander-schranz alexander-schranz Jun 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds for me like we are missing the requirement to spiral/roadrunner-worker package in our own composer.json also. So think we need to add:

"spiral/roadrunner-worker": "^2 || ^3"

also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue with Psalm is not related to this. This package was installed anyway because it is required by roadrunner-php/http. However, spiral/roadrunner-worker is used in this package, so it needs to be added, you are right. I have added it.

alexander-schranz marked this conversation as resolved.
Show resolved Hide resolved

composer-normalize:
name: Composer Normalize
Expand Down
4 changes: 3 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
"nyholm/psr7": "^1.3",
"nyholm/psr7-server": "^1.0",
"react/http": "^1.2",
"spiral/roadrunner": "^2.0",
"spiral/roadrunner-cli": "^2.6",
"spiral/roadrunner-http": "^2.0 || ^3.0",
"spiral/roadrunner-worker": "^2.0 || ^3.0",
"symfony/console": "^5.2 || ^6.0",
"symfony/dependency-injection": "^5.2 || ^6.0",
"symfony/http-foundation": "^5.3 || ^6.0",
Expand Down
4 changes: 3 additions & 1 deletion src/roadrunner-nyholm/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
],
"require": {
"nyholm/psr7": "^1.4",
"spiral/roadrunner": "^2.0",
"spiral/roadrunner-cli": "^2.6",
"spiral/roadrunner-http": "^2.0 || ^3.0",
"spiral/roadrunner-worker": "^2.0 || ^3.0",
"symfony/runtime": "^5.4 || ^6.0 || ^7.0"
},
"require-dev": {
Expand Down
7 changes: 5 additions & 2 deletions src/roadrunner-symfony-nyholm/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@
"require": {
"php": ">=8.0.5",
"nyholm/psr7": "^1.4",
"spiral/roadrunner": "^2.0",
Copy link

@racastellanosm racastellanosm Nov 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about kept this dependency and add the 2023 and 2024 versions in it?

"spiral/roadrunner": "^2 || ^2023 || ^2024",

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See discussions before that is just incorrect and need handled differently.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification!

"spiral/roadrunner-cli": "^2.6",
"spiral/roadrunner-http": "^2.0 || ^3.0",
"spiral/roadrunner-worker": "^2.0 || ^3.0",
"symfony/dependency-injection": "^5.4 || ^6.0 || ^7.0",
"symfony/http-kernel": "^5.4 || ^6.0 || ^7.0",
"symfony/psr-http-message-bridge": "^2.1 || ^6.4 || ^7.0",
"symfony/runtime": "^5.4 || ^6.0 || ^7.0"
},
"require-dev": {
"phpunit/phpunit": "^9.5"
"phpunit/phpunit": "^9.5",
"symfony/console": "^5.4 || ^6.4 || ^7.0"
},
"minimum-stability": "dev",
"prefer-stable": true,
Expand Down
Loading