search

klaviyo / php-klaviyo

PHP wrapper for the Klaviyo API
59 stars 48 forks source link

Refactored code to use php 7.4 + added strict types, phpcsfixer and phpstan checks #56

Open wlodekj opened 3 years ago

wlodekj commented 3 years ago

Hey everyone,

This is my take on improving some of the codebase of klaviyo php sdk. Some key changes in this PR:

1) Update to php 7.4 (added return types, and strict typing to every method) 2) Removed KlaviyoAPI class inheritance in favour of simple DI (now IDE properly autocompletes calls to lists(), metrics(), profiles(), publicAPI() and dataPrivacy() 3) Added phpcsfixer and phpstan
4) New composer jobs added so after composer install running something like this :

composer tests

will execute phpunit tests + cs fixer and phpstan check

and another job added:

composer php:cs

Will automatically fix csfixer errors (new config file added .php_cs.php with all applied rules)

6) Added more tests for Profiles & Metrics (Lists & DataPrivacy still todo)

There is still some work here but wanted to hear your opinion about that. It's probably a release candidate for 3.0.0 version.

Thanks, Jakub

njparadis commented 3 years ago

Thanks for the submission - we do want to retain support for PHP 5.6 for the time being. We will work to include your changes and improvements into future releases of the SDK.

dakorpar commented 2 years ago

@njparadis that doesn't seem like a good option IMHO. https://www.php.net/supported-versions.php

Whoever is using old php version can still use old versions and no one will stop you if U do a small change to release new older versions... In some of libs that I was maintaining I've had 3 active versions and when releasing I would release v1.0.x v2.0.x and v3.0.x no problems there and having such obsolete code is just hard to maintain and will distract developers even from using klaviyo when they're asked.