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

fix(logging): Fix deprecation message in PsrLogger with PHP 8.1 #5006

Merged
merged 3 commits into from
Feb 10, 2022
Merged

fix(logging): Fix deprecation message in PsrLogger with PHP 8.1 #5006

merged 3 commits into from
Feb 10, 2022

Conversation

vntw
Copy link
Contributor

@vntw vntw commented Jan 11, 2022

In PHP 8.1, when implementing Serializable without implementing the magic __serialize/__unserialize methods, a deprecation warning will be generated.

This PR adds these methods and moves the logic as to not duplicate any code but still provide BC for >= PHP 7.2. Tests remain unchanged and working.

The Serializable interface will be removed in PHP 9.0:

As of PHP 8.1.0, a class which implements Serializable without also implementing __serialize() and __unserialize() will generate a deprecation warning.

@vntw vntw requested review from a team as code owners January 11, 2022 16:14
@product-auto-label product-auto-label bot added the api: logging Issues related to the Cloud Logging API. label Jan 11, 2022
In PHP 8.1, when implementing `Serializable` without implementing the magic
`serialize`/`__unserialize` methods, a deprecation warning will be generated.

This PR adds these methods and moves the logic as to not duplicate any code but still provide
BC for >= PHP 7.2. Tests remain unchanged and working.

The Serializable interface will be removed in PHP 9.0:
- RFC: https://wiki.php.net/rfc/phase_out_serializable
- Deprecation Warning docs: https://www.php.net/manual/en/class.serializable.php
>As of PHP 8.1.0, a class which implements Serializable without also implementing __serialize() and __unserialize() will generate a deprecation warning.
Logging/src/PsrLogger.php Outdated Show resolved Hide resolved
Logging/src/PsrLogger.php Outdated Show resolved Hide resolved
@bshaffer bshaffer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 4, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 4, 2022
@vntw
Copy link
Contributor Author

vntw commented Feb 10, 2022

Thanks for the suggestion fixes @bshaffer!

Looking at both failed builds, they seem unrelated to the changes made here since commit fe54725 worked but b246a78 failed (they do the same thing). One failure is also due to CodeSniffer OOM

Would be great to get this merged in the near future since it prevents tools like Infection PHP from working 🙂

@bshaffer bshaffer merged commit 58ff66a into googleapis:master Feb 10, 2022
@vntw vntw deleted the fix-psrlogger-php81 branch February 10, 2022 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the Cloud Logging API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants