-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add support for PHP 7.2 runtime #3736
Conversation
# Licensed to the Apache Software Foundation (ASF) under one or more contributor | ||
# license agreements; and to You under the Apache License, Version 2.0. | ||
|
||
FROM openwhisk/action-php-v7.2:latest |
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.
@@ -0,0 +1,19 @@ | |||
/* |
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.
ansible/files/runtimes.json
Outdated
"default": true, | ||
"deprecated": false, | ||
"image": { | ||
"name": "action-php-v7.2" |
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.
due to #3680, this should specify the prefix as openwhisk and the tag as latest.
settings.gradle
Outdated
@@ -28,6 +28,7 @@ include 'actionRuntimes:swift3.1.1Action' | |||
include 'actionRuntimes:swift4.1Action' | |||
include 'actionRuntimes:javaAction' | |||
include 'actionRuntimes:php7.1Action' | |||
include 'actionRuntimes:php7.2Action' |
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.
tools/build/redo
Outdated
@@ -309,6 +309,11 @@ Components = [ | |||
yaml = False, | |||
gradle = 'actionRuntimes:php7.1Action'), | |||
|
|||
makeComponent('action-php-v7.2', |
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.
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.
both noted PRs are now committed eschewing several files in this PR.
@akrabat
|
What do I need to do in Also, do I need to update |
Rebased against latest master. |
My bad remove redo changes |
whisk.properties is generated file |
Wdyt about giving each of the runtimes their own md file? |
I think reorganising actions.md & reference.md to an md per action makes a lot of sense. |
@rabbah @akrabat @rabbah I think you mentioned you wanted to refactor the basic and Unicode tests to run based on what the runtime manifest States is deployed ? |
Yeah braking the docs per kind runtime is on my personal TODOs There might be duplication, but I think is ok for us(ie maintainers) to deal with the duplication effort (or automation) and end users to benefit of taking a single md file from top to bottom and only deal with the programming language they are dealing and not assume they read the JavaScript examples. |
Codecov Report
@@ Coverage Diff @@
## master #3736 +/- ##
==========================================
- Coverage 74.76% 71.28% -3.48%
==========================================
Files 137 139 +2
Lines 6440 6767 +327
Branches 401 426 +25
==========================================
+ Hits 4815 4824 +9
- Misses 1625 1943 +318
Continue to review full report at Codecov.
|
Version numbers corrected by checking the composer.json in the openwhisk-runtime-php repository.
Updated and rebased for the updated docs structure. |
PG1 3094 ⌛️ |
@@ -1499,6 +1499,8 @@ | |||
"nodejs:8", | |||
"python:2", | |||
"python:3", | |||
"php:7.1", | |||
"php:7.2", |
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'll need to keep this in sync with the runtime manifest in general.
Playground1/3096/ 🔵 |
@@ -1499,6 +1499,8 @@ | |||
"nodejs:8", | |||
"python:2", | |||
"python:3", | |||
"php:7.1", |
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 forgot about swagger 👍
Description
This PR adds PHP 7.2 as a new kind and sets the default PHP runtime to version 7.2.
I have not provided runtime tests here as they are in the runtime-php repository and they are being removed anyway via #3467. I'm not sure which additional tests are needed in this repo.
Related issue and scope
apache/openwhisk-runtime-php#28 needs to be merged and the relevant Docker containers built and uploaded to the Docker Hub first.
My changes affect the following components
Types of changes
Checklist: