-
-
Notifications
You must be signed in to change notification settings - Fork 372
fix(runtimes): docker file command of fpm-dev was misplaced #1008
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
fix(runtimes): docker file command of fpm-dev was misplaced #1008
Conversation
|
|
||
| # Run PHP-FPM | ||
| # opcache.validate_timestamps=1 : cancels the flag in the base configuration so that files are reloaded | ||
| CMD /opt/bin/php-fpm --nodaemonize --fpm-config /opt/bref/etc/php-fpm.conf -d opcache.validate_timestamps=1 --force-stderr |
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.
This was not the right place
Hey that's okay, you're doing great 💪 ❤️ |
|
I've already prepared extension's PR, we have a chance to get ready for first php 8.1 rc relase, september 2nd! |
|
|
||
| FROM without_extensions | ||
|
|
||
| COPY --from=build_dev /opt /opt |
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.
This is a bit sad to copy php-fpm.conf twice, I just wonder if we cannot refine this one to:
| COPY --from=build_dev /opt /opt | |
| COPY --from=build_dev /opt/bref/lib/php/extensions/ /opt/bref/lib/php/extensions/ |
This would allow to remove the COPY instruction below, WDYT?
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.
TBH I have no problem with a duplicate line or something that isn't 100% perfect for a temporary solution (when PHP 8.1 and xdebug support are here I guess this can be all simplified).
|
No problem @shouze, thanks for the follow up PR! Should I merge (since you opened up a question inline)? |
Ok so you can merge, sorry again for the previous mistake 🙏 |
* Adding php 81 for most extensions Related to brefphp/bref#1007 Also related to brefphp/bref#1008, waiting for upfix to be merged * Use 1.2.13 as base bref image * Support xdebug master on PHP 8.1
* Adding php 81 for most extensions Related to brefphp/bref#1007 Also related to brefphp/bref#1008, waiting for upfix to be merged * Use 1.2.13 as base bref image * Support xdebug master on PHP 8.1
@mnapoli I'm confused, my last #1007 refactor was incorrect 😞