search

eversign / eversign-php-sdk

The official eversign PHP SDK
https://eversign.com
MIT License
17 stars 24 forks source link

Allow PHP 8 & require JMS serializer 3 #42

Closed okdewit closed 3 years ago

okdewit commented 3 years ago

After extensively reading the upgrade guides, I can't find any direct compatibility issues with Eversign SDK. Might warrant further testing though 😃

archivelm commented 3 years ago

@okdewit hi, thanks for the contribution once more :D I added one comment, otherwise seems good.

bdw, seems like we have one more request for supporting PHP 8 https://github.com/eversign/eversign-php-sdk/issues/43

okdewit commented 3 years ago

@milanlatinovic Where can I find the comment?

Let me know if there's anything I can do to help this get through :smiley:

archivelm commented 3 years ago

I just added the comment :) I'm thinking if it makes sense to support both existing and 3.x version of jms/serializer

okdewit commented 3 years ago

Updated! :+1:

archivelm commented 3 years ago

Fantastic, I'll take some time and check it out locally, but I do not expect any issues. Will let you know about it asap.

archivelm commented 3 years ago

@okdewit I checked PHP 8 compatibility locally, run linting, did some tests, also checked it manually. I could not recognize any issues. I will approve it and release new version.

I also noticed that in big part of SDK we do not have strong typing (so we do support PHP 7.x and PHP 8.x but majority of the code is PHP 5.x syntax (weak typing with comments). I will make a followup MR to tackle this, if you see it, I would appreciate a review. :D

In short, approved, thanks!

archivelm commented 3 years ago

I made a new release https://github.com/eversign/eversign-php-sdk/releases/tag/1.21.0