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

Predefined functions enhancements #130

Merged
merged 7 commits into from
Apr 19, 2023

Conversation

gmazzap
Copy link
Collaborator

@gmazzap gmazzap commented Dec 12, 2022

Add stubs for wp_validate_boolean() and wp_slash().

See #125

@gmazzap gmazzap requested a review from jrfnl December 12, 2022 18:25
Copy link
Collaborator

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Verified against the WP functions: ✅
Docs: ❌ - I'd expect these need to be added to the "Generic functions" list in the wordpress-tools.md file.
Tests: ❌ - I don't see any (availability) tests for this functionality.

@paulshryock
Copy link
Contributor

paulshryock commented Mar 7, 2023

I created a PR #134 into this branch which adds some test assertions for wp_validate_boolean. Based on how the existing tests are setup and named, this seemed like the right approach. Wasn't sure if this would bring this PR one step closer towards being ready to merge.

If this is the right approach, and that PR gets merged, then I could similarly add some test assertions for wp_slash and the missing documentation. But I wanted to get some feedback on the approach I took before spending more time on that.

@gmazzap or @jrfnl, if you get a chance to take a look at #134, please let me know if that can be merged, or if you'd recommend any different approach for adding these tests.

…test-assertions

Add wp_validate_boolean test assertions
@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.35 ⚠️

Comparison is base (d9ff201) 92.25% compared to head (7f037a5) 91.90%.

❗ Current head 7f037a5 differs from pull request most recent head 202d84c. Consider uploading reports for the commit 202d84c to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #130      +/-   ##
============================================
- Coverage     92.25%   91.90%   -0.35%     
  Complexity      312      312              
============================================
  Files            28       28              
  Lines           710      828     +118     
============================================
+ Hits            655      761     +106     
- Misses           55       67      +12     

see 20 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@gmazzap
Copy link
Collaborator Author

gmazzap commented Apr 19, 2023

Thanks @jrfnl for the review and sanity check, as usual.

Thanks @paulshryock for the PR, approved your PR and merged it.

I added docs and test for wp_slash (@paulshryock did tests for wp_validate_boolean). I think this is good to merge now.

@gmazzap gmazzap merged commit 1a63b6f into master Apr 19, 2023
@gmazzap gmazzap deleted the feature/predefined-functions-enhancements branch April 19, 2023 07:27
@jrfnl jrfnl added this to the 2.x Next milestone May 1, 2023
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.

4 participants