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

TGR-152: PHP 8 update #16

Open
wants to merge 58 commits into
base: master
Choose a base branch
from
Open

TGR-152: PHP 8 update #16

wants to merge 58 commits into from

Conversation

jberube-nypl
Copy link
Member

@jberube-nypl jberube-nypl commented Sep 23, 2024

The goal of this PR is to update the code to work with PHP 8.3.

Slim - This framework library needed to be upgraded from v3 to v4 for PHP 8 compatibility. This required many changes to the codebase to implement its API changes. The Aura/DI library was also added since the new version of Slim requires it.

Swagger PHP - the dependent library changed its name from Swagger to OpenAPI, so docblock comments have been changed from SWG to OA.

Avro PHP - this library was forked from version 1.8.1 and the files were included in this repo. The modifications were just a few simple changes that allowed schema arrays to have integers for keys. The base version, 1.8.1 was only compatible with PHP 5.3, so I got a newer version from https://github.com/nealio82/avro-php and made the same modifications and placed it in the lib folder. I was hoping to use Composer to include the new Avro dependency and use a patch for the modifications, but since the PHP Microservice Starter is itself used as a Composer dependency, it isn't allowed have patches on its own dependencies.

Updated Libraries:
slim/slim: 3.5.0 => 4.14.0
slim/pdo: 1.9.9 => faapz/pdo 2.2.0
monolog/monolog: 1.23.0 => 3.7.0
danielstjules/stringy: 2.3.2 => 3.1
zircote/swagger-php: 2.0.7 => 4.10.6
aws/aws-sdk-php: 3.31.4 => 3.319.4
vlucas/phpdotenv: 2.4.0 => 5.6.1
guzzlehttp/guzzle: 6.2.3 => 7.9.2

jberube-nypl and others added 30 commits July 10, 2024 10:22
@jberube-nypl jberube-nypl changed the title PHP 8 update TGR-152: PHP 8 update Dec 23, 2024
@jberube-nypl jberube-nypl requested a review from nonword December 23, 2024 13:49
@@ -0,0 +1 @@
This is a fork of https://github.com/nealio82/avro-php. Changes were made to allow schema arrays to have integer keys.
Copy link
Member

Choose a reason for hiding this comment

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

Can we note the specific version you copied in? And where/how you patched? We maybe also want to link to this README from the top-level README.

@@ -43,7 +43,7 @@ protected static function setCachedString($type = '', $originalString = '', $str
*
* @return string
*/
protected static function runStringy($type = '', $string = '', callable $stringy)
protected static function runStringy($type, $string, callable $stringy)
Copy link
Member

Choose a reason for hiding this comment

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

Defaults removed because they're enforced elsewhere or we want things to fail when the params are null?

@@ -47,12 +47,12 @@ public static function initialize($configDirectory = '')
*/
public static function get($name = '', $defaultValue = null, $isEncrypted = false)
{
if (getenv($name) !== false) {
if (!empty($_ENV[$name])) {
Copy link
Member

Choose a reason for hiding this comment

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

Does this not raise a warning/error if $name isn't set in $_ENV?

@@ -155,7 +155,7 @@ protected static function loadConfiguration()
}

if (file_exists(self::getConfigDirectory() . '/' . self::GLOBAL_ENVIRONMENT_FILE)) {
$dotEnv = new Dotenv(self::getConfigDirectory(), self::GLOBAL_ENVIRONMENT_FILE);
$dotEnv = Dotenv::createImmutable(self::getConfigDirectory(), self::GLOBAL_ENVIRONMENT_FILE);
Copy link
Member

Choose a reason for hiding this comment

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

Just curious - are we intentionally using Immutable over Mutable because there are specific env vars we anticipate that we don't want to overwrite?

public function __construct()
public function __construct(
InjectionFactory $injectionFactory,
ContainerInterface $delegateContainer = null)
Copy link
Member

Choose a reason for hiding this comment

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

What do these added params do?

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.

3 participants