Skip to content
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

PHP version update #266

Merged
merged 1 commit into from
Oct 21, 2021
Merged

PHP version update #266

merged 1 commit into from
Oct 21, 2021

Conversation

Shikachuu
Copy link
Contributor

@Shikachuu Shikachuu commented Aug 21, 2021

Signed-off-by: Shikachuu zcmate@gmail.com

Description

To mostly maintain backward compatibility I update the images to PHP 7.4 to keep the php7 name.
Update PHP version everywhere to 7.4, changed composer flag to :1 to always use the most up-to-date composer:1 version.
(I also added type hints to the Handler class, but that's not the main topic of this pull request, so it can be removed if there is a need.)

Motivation and Context

The reason behind this PR is that PHP 7.2 is EOL since 2020 November and composer was way down in security patches.

Which issue(s) this PR fixes

How Has This Been Tested?

Since it is just a version bump in the Dockerfile I just ran a Docker build command to check for complications.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Version change (see: Impact to existing users)

Impact to existing users

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

…always use the most up-to-date composer:1 version.

Signed-off-by: Shikachuu <zcmate@gmail.com>
@Shikachuu Shikachuu mentioned this pull request Aug 21, 2021
2 tasks
*/
public function handle($data)
public function handle(string $data): string
Copy link
Member

Choose a reason for hiding this comment

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

Before you made this explicitly into a string, what kind of data was being passed in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was/is string, it gets data directly from file_get_contents("php://stdin"), which is read the entire stdin into a string or return false on failure (in case of an explicit php://stdin should never happen).
https://www.php.net/manual/en/function.file-get-contents

The explicit string type is only there to aid the developer, nothing fancy.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for clarifying.

Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

LGTM

@alexellis alexellis merged commit 6e2657c into openfaas:master Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants