-
Notifications
You must be signed in to change notification settings - Fork 423
doc(runtime): mention Symfony 7.4 native support for worker mode #1668
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
Conversation
|
that's sweet! |
|
In my opinion, we should wait for the stable release of Symfony 7.4 and remove the outdated documentation at this point. |
|
It could be useful to have a note to help people upgrading rather than completely remove the current doc? |
|
I think a "starting with symfony 7.4" note is totally fine. Or we just keep the PR on hold until 7.4 releases. |
63ca0fd to
bc07241
Compare
bc07241 to
b3c7099
Compare
|
Let's close and wait for 7.4 as stated in #1668 (comment). |
|
I would keep this open/drafted so we don't forget about it. |
|
Let's keep it in draft then 🙂 |
docs/fr/worker.md
Outdated
| ``` | ||
|
|
||
| > [!TIP] | ||
| > À partir de Symfony 7.4, la dépendance `runtime/frankenphp-symfony` n'est plus nécessaire, car Symfony Runtime prend en charge nativement le mode worker de FrankenPHP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the value of APP_RUNTIME env var must be changed ?
Also it seems weird to read of FRANKENPHP_CONFIG to activate worker as it is now the default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting APP_RUNTIME will not be necessary anymore.
Worker mode is not the default, not sure how you got that impression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ve taken a shorcut saying this I suppose
The « recommended » way for running symfony and docker is via the repo https://github.com/dunglas/symfony-docker/blob/main/frankenphp/Caddyfile and its caddyfile declare worker
so as this PR talks about symfony and runtime etc, i was asking if it is not weird to read this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting APP_RUNTIME won't be necessary but as long as we support version without this feature on the Symfony side, I guess we'll just keep it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still say that "the following changes are necessary if you use an older Symfony version than 7.4". Symfony 7.4+ users can skip the whole page and shouldn't be setting APP_RUNTIME
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your comments all
also what about mentioning the FRANKENPHP_LOOP_MAX (at least) ?
I can provide a PR for both env vars
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. please create one targeting this PR branch!
|
Symfony 7.4 is out! @dunglas do you still think that we should remove entirely mention of the additional runtime? |
I think we'd all like to, but we really can't. Users with Symfony <= 7.3 would not be happy and we can't expect everyone to adopt immediately. I think it's fair to remove it when Symfony 7.3 goes out of support, up until that point we have to keep it. |
b3c7099 to
b508831
Compare
|
Ready @php/frankenphp-collaborators 🙂 |
b508831 to
0c0147e
Compare
henderkes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great wording!
|
Think we forgot about this one, linter is now happy. |
Fixes symfony/symfony-docs#21099, related to symfony/symfony#60503