Skip to content

Conversation

@soundslocke
Copy link

@soundslocke soundslocke commented Jul 5, 2023

Fixes

Without these fixes you will encounter PHP messages similar to these when using modern versions of PHP (8.1+):

substr(): Passing null to parameter #1 ($string) of type string is deprecated
file: trolley/core/lib/Trolley/Configuration.php
line: 471

and

Return type of Trolley\\ResourceCollection::current() should either be compatible with Iterator::current(): mixed, or the #[\\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
file: trolley/core/lib/Trolley/ResourceCollection.php
line: 59

...plus a few others related to the various other Iterator methods in ResourceCollection.php.

Background

For a while we've been using an internally patched version of the older 2.x Trolley PHP SDK (the version previously using the PaymentRails namespace).

Given the recent library upgrades here and the new published versions, complete with new Trolley namespace, we decided to upgrade to the latest SDK here. In doing so we noticed these issues again, so I decided to go ahead and fork and create this PR!

Notes

  1. In the future you might consider adding return types within ResourceCollection.php as well and remove the #[\ReturnTypeWillChange] attributes which just suppress the notices. This may involve a little more work to ensure proper return types for each method.

  2. The substr() deprecation fix we went with uses null coalescing and this requires PHP 7.0 or higher, so I've updated that dependency in composer.json and updated the readme.

Checklist

  • I acknowledge that all my contributions will be made under the project's license

@barnett barnett requested a review from Aman-Aalam July 11, 2023 13:30
@Aman-Aalam Aman-Aalam changed the base branch from master to php-ver-update July 11, 2023 22:28
@Aman-Aalam
Copy link
Contributor

This will need a larger amount of work, hence merging it in a branch which we can pick up in next sprint.

@Aman-Aalam Aman-Aalam merged commit 595965b into trolley:php-ver-update Jul 11, 2023
@soundslocke
Copy link
Author

soundslocke commented Jul 11, 2023

Gotcha, thanks for the update @Aman-Aalam. I had a hunch it would entail more, but we didn't want to get too far into it ourselves and thus the quick patch and PR here. We'll be looking forward to your more complete or ideal PHP 8.1 compatibility changes (and we'll continue using our fork in the meantime).

@Aman-Aalam
Copy link
Contributor

Thanks a lot for the contribution, @soundslocke .
Yes, we'll be eyeing for the latest stable/LTS PHP version.

@soundslocke soundslocke mentioned this pull request Oct 19, 2023
7 tasks
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.

2 participants