Skip to content

fix: address review issues in FrankenPHP Docker support (#8213)#8221

Merged
DawoudIO merged 1 commit intocopilot/test-churchcrm-with-frankenphpfrom
fix/frankenphp-review
Mar 9, 2026
Merged

fix: address review issues in FrankenPHP Docker support (#8213)#8221
DawoudIO merged 1 commit intocopilot/test-churchcrm-with-frankenphpfrom
fix/frankenphp-review

Conversation

@DawoudIO
Copy link
Contributor

@DawoudIO DawoudIO commented Mar 9, 2026

Follow-up fixes for #8213 based on code review.

Changes

  • Dockerfile.churchcrm-frankenphp: Use mv instead of cp for php.ini-production — avoids leaving the unused template in the image layer, consistent with Dockerfile.churchcrm-fpm-php8
  • docker/README.md: Convert setext --- heading to ATX ### style for consistency with the rest of the file
  • docker/README.md: Add missing newline at end of file

Test plan

  • Verify docker build -f Dockerfile.churchcrm-frankenphp --target prod . succeeds
  • Confirm php.ini-production is absent and php.ini is present in the built image
  • Confirm README renders correctly on GitHub

🤖 Generated with Claude Code

- Use mv instead of cp for php.ini-production to avoid leaving
  unused template in the image layer (matches Dockerfile.churchcrm-fpm-php8)
- Fix README heading style: setext --- to ATX ### for consistency
- Add missing newline at EOF in README.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 9, 2026 03:13
@DawoudIO DawoudIO requested a review from a team as a code owner March 9, 2026 03:13
@DawoudIO DawoudIO requested review from DAcodedBEAT, MrClever, bigtigerku, grayeul and respencer and removed request for a team March 9, 2026 03:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Follow-up adjustments to the FrankenPHP Docker support added in #8213, aiming to align the Docker image layering and Docker documentation formatting with existing patterns in the repo.

Changes:

  • Switch php.ini-production setup in the FrankenPHP Dockerfile from cp to mv to avoid leaving the template in the image layer.
  • Update the FrankenPHP section heading in docker/README.md and ensure the file ends with a newline.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
docker/README.md Adjusts FrankenPHP section heading formatting and fixes EOF newline.
docker/Dockerfile.churchcrm-frankenphp Uses mv for php.ini-productionphp.ini to reduce leftover artifacts in the image layer.


Self-Hosted / Production (FrankenPHP)
---
### Self-Hosted / Production (FrankenPHP)
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The FrankenPHP section heading was changed from a setext-style H2 to an ATX H3 ("###"), which breaks the document hierarchy: the subsequent "### Why FrankenPHP..." and "### Quick start" headings are now the same level instead of being nested under the FrankenPHP section. Consider making this heading an H2 (e.g., setext like the nginx section, or use "##") so the subheadings remain properly nested.

Suggested change
### Self-Hosted / Production (FrankenPHP)
## Self-Hosted / Production (FrankenPHP)

Copilot uses AI. Check for mistakes.
@DawoudIO DawoudIO merged commit 68f61d3 into copilot/test-churchcrm-with-frankenphp Mar 9, 2026
6 checks passed
@DawoudIO DawoudIO deleted the fix/frankenphp-review branch March 9, 2026 14:00
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.

2 participants