Skip to content

add php56 support + jinja2 templating to php modules#30

Merged
Farouk-Lamri merged 25 commits intomasterfrom
feat/php-56-support
Sep 11, 2020
Merged

add php56 support + jinja2 templating to php modules#30
Farouk-Lamri merged 25 commits intomasterfrom
feat/php-56-support

Conversation

@RSchumacher13
Copy link
Contributor

#Add PHP 56 support + jinja2 templating to php-modules definition

  • Doesn't alter the original behavior of the program, however, /runtime-containers/components/php_modules/deb/php_modules.j2 replaces /runtime-containers/components/php_modules/deb/php_modules.dtc as asked in DTC Jinja template #17
  • Adds PHP 56 runtime
  • Adapts Bats test to be performed on < PHP 7 environments

@RSchumacher13 RSchumacher13 linked an issue Sep 7, 2020 that may be closed by this pull request
Farouk-Lamri
Farouk-Lamri previously approved these changes Sep 9, 2020
Copy link
Collaborator

@Pierozi Pierozi left a comment

Choose a reason for hiding this comment

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

I think you will have issue regarding the deployment of runtime php_custom.
check the ci script https://github.com/continuousphp/runtime-containers/blob/master/bin/ci.sh#L65-L69

The name of the image build locally and the tag does not match due to the fact that the script was design only for one runtime... must adapt it but still pushed to the same ECR in case of php...

@RSchumacher13
Copy link
Contributor Author

RSchumacher13 commented Sep 10, 2020

I think you will have issue regarding the deployment of runtime php_custom.
check the ci script https://github.com/continuousphp/runtime-containers/blob/master/bin/ci.sh#L65-L69

The name of the image build locally and the tag does not match due to the fact that the script was design only for one runtime... must adapt it but still pushed to the same ECR in case of php...

Should be ok now. Conversion to semantic versioning is now done when building the image, and the ci script is using the $runtime variable instead of hardcoded php. php is now usef for > 7 and php for 5.6.40 (obtained from devilbox/php-fpm:5.6-base which is running php 5.6.40).

FYI: I performed tests in order to consider php < 5.6.40 as a 'normal' php runtime, which is loading another image than php > 7.0, this would make more sense for the users and would avoid having two php runtimes named differently.

Farouk-Lamri
Farouk-Lamri previously approved these changes Sep 10, 2020
fdewinne
fdewinne previously approved these changes Sep 10, 2020
Pierozi
Pierozi previously approved these changes Sep 11, 2020
@RSchumacher13 RSchumacher13 requested review from Pierozi and removed request for Pierozi September 11, 2020 08:54
@Farouk-Lamri Farouk-Lamri merged commit d4c5acf into master Sep 11, 2020
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.

DTC Jinja template

4 participants