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

[ticket/17097] Fix PHP 8.2 deprecation warnings #6478

Merged
merged 1 commit into from May 29, 2023

Conversation

m-ober
Copy link
Contributor

@m-ober m-ober commented Apr 16, 2023

Checklist:

  • Correct branch: master for new features; 3.3.x for fixes
  • Tests pass
  • Code follows coding guidelines: master and 3.3.x
  • Commit follows commit message format

Tracker ticket (set the ticket ID to your ticket ID):

https://tracker.phpbb.com/browse/PHPBB3-17097

@m-ober
Copy link
Contributor Author

m-ober commented Apr 16, 2023

Maybe it would be better to add role_cache as property to the auth class, but I don't know if this was left out intentional.

In case of phpbb_root_path it looks like it was simply forgotten.

@m-ober
Copy link
Contributor Author

m-ober commented Apr 18, 2023

We could also replace the ReturnTypeWillChange attributes with the actual return type, except for offsetGet, which has a mixed return type that is only supported by PHP >=8.0.

@m-ober
Copy link
Contributor Author

m-ober commented May 23, 2023

As this question came up in chat, Attributes are backwards compatible as long as they are on a separate line. Then, they are just parsed as comments by PHP <8.0.

@marc1706 marc1706 added this to the 3.3.11 milestone May 23, 2023
@marc1706
Copy link
Member

Maybe it would be better to add role_cache as property to the auth class, but I don't know if this was left out intentional.

In case of phpbb_root_path it looks like it was simply forgotten.

It actually does not need to be a class property as it's never used as such. In master, it was changed to no longer use this:
774c609#diff-d3edffaad6045eb7577d57cbb613c8903eb4d239c9f41aeaabfd1218ebdc596c

@marc1706
Copy link
Member

Thanks for the pull request. Rest of the changes look fine. Do you want to apply the above changes? Otherwise it's also fine with me to skip them.

@marc1706 marc1706 closed this May 23, 2023
@marc1706 marc1706 reopened this May 23, 2023
@marc1706
Copy link
Member

Closed and reopened the PR to trigger builds. ;)

@m-ober
Copy link
Contributor Author

m-ober commented May 23, 2023

@marc1706 Thanks for the review! I have cherry-picked the changes from master (for the changes regarding the $role_cache variable only). Also, I have added the #[\ReturnTypeWillChange] attribute to the same methods in the config class. I never got a deprecation notice from this class, but as it is also implementing the ArrayAccess interface, it could trigger the same notices.

@m-ober
Copy link
Contributor Author

m-ober commented May 26, 2023

Any idea why the "Check commit message" step is failing? The error message is not very helpful.

@rubencm
Copy link
Member

rubencm commented May 27, 2023

The commit should be like this

[ticket/17097] Fix PHP 8.2 deprecation warnings

PHPBB3-17097

@m-ober
Copy link
Contributor Author

m-ober commented May 27, 2023

@rubencm Thanks, fixed.

@marc1706 marc1706 merged commit 75dcbea into phpbb:3.3.x May 29, 2023
41 checks passed
@marc1706
Copy link
Member

Thanks for your contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants