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 get comment 8 0 #714

Merged
merged 3 commits into from Sep 16, 2021
Merged

Conversation

bahiirwa
Copy link
Collaborator

@bahiirwa bahiirwa commented Apr 25, 2021

Pr fixes the PHP 8.0 compatibility issues and closes ClassicPress/ClassicPress-v1#80 (Issue is detailed)

PR constains Merge conflicts and Backport changesets

as #comment:2 suggests.

How is tested?

After the patch, my admin area has not issue as in screenshot.
Screenshot 2021-04-25 at 20 07 21

Yet pre-patch, we had
Screenshot 2021-04-25 at 18 05 38

SergeyBiryukov and others added 3 commits April 25, 2021 18:29
…reference, value given" error in `WP_Comment_Query::get_comments()`.

The WP native `get_comment()` function expects the first argument `$comment` to be passed by reference.

The PHP `array_map()` function, however, passes by value, not by reference, resulting in an "arguments must be passed by reference, value given" error.

The PHP native `array_walk()` function does pass by reference. Using this prevents the error on PHP 8 and maintains the existing behaviour on PHP < 8.

WP:Props jrf.
See https://core.trac.wordpress.org/ticket/50913.

Conflicts:
- src/wp-includes/class-wp-comment-query.php

---

Merges https://core.trac.wordpress.org/changeset/48838 / WordPress/wordpress-develop@29bfc56ca2 to ClassicPress.
… `get_comment()` definition.

This fixes a PHP 8 "argument must be passed by reference, value given" error when using `array_map( 'get_comment', ... )`.

Object variables in PHP 5+ contain a reference to the object, and it's the reference that's passed around.

Note: This reverts https://core.trac.wordpress.org/changeset/48838, which is now redundant.

Follow-up to a similar change for `get_post()` in https://core.trac.wordpress.org/changeset/21572.

See https://core.trac.wordpress.org/ticket/50913.

---

Merges https://core.trac.wordpress.org/changeset/48961 / WordPress/wordpress-develop@875acd7b37 to ClassicPress.
@bahiirwa bahiirwa added scope: php8 PHP 8 compatibility status: community reviewed Reviewed by a community member. labels Apr 25, 2021
@bahiirwa bahiirwa added this to the 1.4.0-rc1 milestone Jul 11, 2021
@bahiirwa bahiirwa added the type: wp backport Related to backporting changes from WordPress. label Jul 20, 2021
@ClassyBot
Copy link
Contributor

This pull request has been mentioned on ClassicPress Forums. There might be relevant details there:

https://forums.classicpress.net/t/work-towards-release-1-4-0/3305/1

@mattyrob mattyrob mentioned this pull request Sep 16, 2021
1 task
Copy link
Contributor

@nylen nylen left a comment

Choose a reason for hiding this comment

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

There is a small problem with this PR: the commit marked "Fix Merge Conflicts for 48838" actually reverts r48838 instead of fixing the conflicts with it.

However, the next change (backporting r48961) would undo r48838 anyway, so the PR ends up with the correct result when all changes are taken into account.

Therefore this PR is good to ship.

@mattyrob mattyrob merged commit 909c7de into ClassicPress:develop Sep 16, 2021
@mattyrob
Copy link
Collaborator

Reviewed during Slack meeting and agreed for commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: php8 PHP 8 compatibility status: community reviewed Reviewed by a community member. type: wp backport Related to backporting changes from WordPress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP 8.0: "argument must be passed by reference, value given" in WP_Comment_Query::get_comments()
5 participants