Skip to content
This repository was archived by the owner on Aug 18, 2023. It is now read-only.

Conversation

@wlodekj
Copy link

@wlodekj wlodekj commented May 10, 2021

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)

  1. 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
Copy link

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
Copy link

dakorpar commented Jun 8, 2022

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants