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

Php 8.4 deprecations fix #90

Closed
wants to merge 2 commits into from

Conversation

acelaya
Copy link

@acelaya acelaya commented Oct 29, 2024

This PR solves a deprecation warning reported when using this library in PHP 8.4

The reason for this warning is a protected method from the Factory class which has two arguments implicitly marked as nullable by having a null default value.

protected static function rangeFromBoundaryAddresses(AddressInterface $from = null, AddressInterface $to = null)

That is deprecated in PHP 8.4, and will error in PHP 9.0 and above.

One possible approach to solve that is explicitly marking the arguments as nullable (eg. ?AddressInterface $from = null instead of AddressInterface $from = null), but that requires PHP 7.1, and this library supports older versions.

Since the method where this error is reported was protected and being called only from one place, I moved the logic there instead, removing the method entirely.

This can be a breaking change though, if anyone was extending from Factory and invoking this method from their own class, so perhaps this requires a major version bump.

Additionally, this PR also adds PHP 8.3 and 8.4 to the tests workflow matrix.

Closes #89

@acelaya acelaya mentioned this pull request Oct 29, 2024
@acelaya acelaya changed the title Php 8.4 deprecations Php 8.4 deprecations fix Oct 29, 2024
@coveralls
Copy link

Coverage Status

coverage: 98.621% (-0.001%) from 98.622%
when pulling beff3a9 on acelaya-forks:php-8.4-deprecations
into 125ab84 on mlocati:main.

@acelaya
Copy link
Author

acelaya commented Oct 29, 2024

Coverage Status

coverage: 98.621% (-0.001%) from 98.622% when pulling beff3a9 on acelaya-forks:php-8.4-deprecations into 125ab84 on mlocati:main.

This is probably a false negative, caused simply by the fact that there's now less total lines.

@mlocati
Copy link
Owner

mlocati commented Oct 29, 2024

#91 is a less intrusive alternative, IMHO

@acelaya
Copy link
Author

acelaya commented Oct 29, 2024

Oh! That didn't occur to me, and it's backwards compatible.

I'll close mine.

Thanks!

@acelaya acelaya closed this Oct 29, 2024
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.

Deprecations with PHP 8.4
3 participants