Skip to content

Conversation

@ideag
Copy link

@ideag ideag commented Mar 23, 2022

Fixes #122 - serialization warnings in PHP 8.*

gitlost and others added 30 commits November 17, 2017 23:45
…porting

Update scaffolded tests to enable error reporting
* Regenerates README.md
* Adds `.github/settings.yml` for description and label management.
* Updates `.github/PULL_REQUEST_TEMPLATE`
Update scaffolded README and GitHub configuration
Restrict some search-replace tests to wp_posts to avoid wp_options cl…
Add `--skip-tables=<tables>` argument to exclude tables when performing search-replace
Remove unnecessary `array_diff()`; skipping within loop is clearer
Convert search-replace subcommand help summaries to use third-person singular verbs.
@ideag ideag requested a review from a team as a code owner March 23, 2022 21:19
@rwagner00
Copy link

Will confirm that it resolved #122 for me and works as intended.

}

$unserialized = is_string( $data ) ? @unserialize( $data ) : false;
$unserialized = is_serialized( $data ) ? @unserialize( $data ) : false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally seems fine. Could we get a test case for it?

Also, let's internalize is_serialized() to a private method on this class. As far as I can tell, we don't have any other WordPress-specific dependencies in this class, so better to keep things that way for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, during a search-replace, WP might potentially be broken, so it's best to copy the function over.

@schlessera schlessera force-pushed the fix-serialize-warnings-php8 branch from dc379db to a758b68 Compare September 1, 2022 16:04
@danielbachhuber
Copy link
Member

Proceeding with wp-cli/wp-cli#5594 for this repository. I've captured this PR to https://gist.github.com/danielbachhuber/6c429bfdd43cae3ddb72559ad1f54446 in case this PR is auto-closed or broken in some way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warning: unserialize() expects parameter 1 to be string, array given