-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add wrapper wp_get_object_vars() #3607
base: trunk
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @cadic! I've left some feedback in this review to tidy things up and meet Core standards/convention. 🙂 Feel free to ping me for a re-review.
* @since 6.3.0 | ||
* | ||
* @param object $object An object instance. | ||
* @return array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @return array | |
* @return array The properties of the given object. |
* @dataProvider data_test_wp_comment_null_byte | ||
* @ticket 52738 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @dataProvider data_test_wp_comment_null_byte | |
* @ticket 52738 | |
* @ticket 52738 | |
* | |
* @covers WP_Comment::__construct | |
* | |
* @dataProvider data_test_wp_comment_null_byte | |
* | |
* @param stdClass $comment_data The comment's data. | |
* @param string $expected The expected comment content. |
- See this ticket for the order of annotations in the test suite.
- A
@covers
annotation should be used. - Test method parameters should be documented.
🔢 Applies elsewhere in the PR.
$this->assertSame( $comment->comment_content, $expected ); | ||
} | ||
|
||
public function data_test_wp_comment_null_byte() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public function data_test_wp_comment_null_byte() { | |
/** | |
* Data provider. | |
* | |
* @return array[] | |
*/ | |
public function data_test_wp_comment_null_byte() { |
🔢 Applies elsewhere in the PR.
// '8)' => "\xf0\x9f\x98\x8e", | ||
// '8)' => "\xf0\x9f\x98\x8e", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an unrelated change.
/* | ||
First we check if the DOMDocument class exists. If it does not exist, then we cannot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an unrelated change.
|
||
public function data_test_wp_comment_null_byte() { | ||
return array( | ||
array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Datasets should have named keys so that it's immediately clear what's being tested, and that passing --testdox
to PHPUnit makes it clear what is being tested.
For example:
'a key that is only a NUL byte'
'a key starting with a NUL byte'
'a key ending with a NUL byte'
🔢 Applies elsewhere in the PR.
Trac ticket: https://core.trac.wordpress.org/ticket/52738
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.