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

[EXAMPLE] Demonstrate current behavior for update_option #5250

Closed

Conversation

joemcgill
Copy link
Member

@joemcgill joemcgill commented Sep 19, 2023

This adds only unit tests for #5139 without changing any current behavior in order to demonstrate the current behavior against which any new fixes can be compared.

Trac ticket: https://core.trac.wordpress.org/ticket/22192


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.

array( false, 0 ),
array( false, 0.0 ),
array( false, false ),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of checking all truthy/falsey, should we add cases for array (empty and populated), object (always truthy), and null (always falsey)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly! I was assuming we wanted to limit the use cases to only scalar values. Since arrays and null values are not scalar, they may need to be handled differently. Worth confirming though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a set of non-scalar use cases in cf71d2c. Interesting to see which loosey values result in an update and which do not.

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thanks @joemcgill, this looks good to commit IMO.

@felixarntz
Copy link
Member

@felixarntz felixarntz closed this Sep 21, 2023
@joemcgill joemcgill deleted the example/22192-current-behavior branch October 11, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants