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

Enhancement/3157 save idea info #3320

Merged
merged 8 commits into from
May 13, 2021
Merged

Conversation

eugene-manuilov
Copy link
Collaborator

@eugene-manuilov eugene-manuilov commented May 7, 2021

Summary

Addresses issue #3157

Relevant technical choices

Checklist

  • My code is tested and passes existing unit tests.
  • My code has an appropriate set of unit tests which all pass.
  • My code is backward-compatible with WordPress 4.7 and PHP 5.6.
  • My code follows the WordPress coding standards.
  • My code has proper inline documentation.
  • I have added a QA Brief on the issue linked above.
  • I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Sorry, something went wrong.

@google-cla google-cla bot added the cla: yes label May 7, 2021
Copy link
Collaborator

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

Seems good, but I'm curious about a misspelling in the test case.

Also: can you add a QA brief? I know in this case it's essentially "Make sure tests pass", but better to leave a minimal one than an empty one 🙂

protected function assertPostMetaHasValue( $post_id, $meta_key, $meta_value ) {
$meta = $this->queryPostMeta( $post_id, $meta_key );
$this->assertNotNull( $meta );
$this->assertEquals( $meta_value, $meta['meta_value'], "Failted to assert that post $post_id has \"$meta_key\" meta with \"$meta_value\" value." );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$this->assertEquals( $meta_value, $meta['meta_value'], "Failted to assert that post $post_id has \"$meta_key\" meta with \"$meta_value\" value." );
$this->assertEquals( $meta_value, $meta['meta_value'], "Failed to assert that post $post_id has \"$meta_key\" meta with \"$meta_value\" value." );

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like a misspelling here, but then it did pass. What's causing this to emit "failted"? I couldn't find it in our code, but is it worth noting why that… typo appears? 🤷🏻‍♂️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tofumatt the last parameter is just a friendly message that will be displayed if the first two parameters don't equal.

Co-authored-by: Matthew Riley MacPherson <[email protected]>
@tofumatt tofumatt merged commit cf3e891 into develop May 13, 2021
@tofumatt tofumatt deleted the enhancement/3157-save-idea-info branch May 13, 2021 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants