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

Adjust tests for PHP 8.3 #64

Merged
merged 2 commits into from
Jan 30, 2024
Merged

Adjust tests for PHP 8.3 #64

merged 2 commits into from
Jan 30, 2024

Conversation

athos-ribeiro
Copy link
Contributor

As of PHP 8.3.0, calling ReflectionProperty::setValue with a single argument is deprecated. See [1] for further reference.

[1] https://www.php.net/manual/en/reflectionproperty.setvalue.php

As of PHP 8.3.0, calling ReflectionProperty::setValue with a single
argument is deprecated. See [1] for further reference.

[1] https://www.php.net/manual/en/reflectionproperty.setvalue.php
@malarzm
Copy link
Member

malarzm commented Jan 26, 2024

Thanks for the PR! PHP is recommending another way now though:

Calling this method with a single argument is deprecated, ReflectionClass::setStaticPropertyValue() should be used instead to modify static properties.

Shall we use that method given our properties are static?

Also, would you mind adding 8.3 to

php-versions: '["7.1", "7.2", "7.3", "7.4", "8.0", "8.1", "8.2"]'

so we actually run tests against 8.3? :)

@derrabus
Copy link
Member

Shall we use that method given our properties are static?

Given that we're not setting a public property here, setStaticPropertyValue() won't work for us on PHP versions below 7.4.

https://3v4l.org/oI1rb

I think, the proposed change is fine.

@athos-ribeiro
Copy link
Contributor Author

so we actually run tests against 8.3?

Done.

And based on @derrabus comment, I suppose I should not change this to use ReflectionClass, right?

Thanks for the reviews :)

@malarzm malarzm added this to the 1.1.3 milestone Jan 30, 2024
@malarzm malarzm added the bug Something isn't working label Jan 30, 2024
@malarzm malarzm merged commit dfbaa3c into doctrine:1.1.x Jan 30, 2024
13 checks passed
@malarzm
Copy link
Member

malarzm commented Jan 30, 2024

Yup, @derrabus was totally right and I was not :) Thanks for the PR @athos-ribeiro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants