Skip to content

CS/QA: no need for is_null()#22128

Merged
jrfnl merged 1 commit intotrunkfrom
JRF/CS/no-is-null
Mar 21, 2025
Merged

CS/QA: no need for is_null()#22128
jrfnl merged 1 commit intotrunkfrom
JRF/CS/no-is-null

Conversation

@jrfnl
Copy link
Copy Markdown
Contributor

@jrfnl jrfnl commented Mar 21, 2025

Context

  • Code consistency

Summary

This PR can be summarized in the following changelog entry:

  • Code consistency.

Relevant technical choices:

No need for a function call when a direct comparison will do. The is_null() function should generally only be used as a callback.

Also no need to do both an isset() as well as a is_null(). isset() will already return false if the value is null, so that is just a duplicate check.

Includes replacing some unsafe type checks with safer ones.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • N/A

No need for a function call when a direct comparison will do. The `is_null()` function should generally only be used as a callback.

Also no need to do both an `isset()` as well as a `is_null()`. `isset()` will already return `false` if the value is `null`, so that is just a duplicate check.

Includes replacing some unsafe type checks with safer ones.
@jrfnl jrfnl added yoast cs/qa changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog labels Mar 21, 2025
@jrfnl jrfnl added this to the 24.9 milestone Mar 21, 2025
@jrfnl jrfnl merged commit e034cb1 into trunk Mar 21, 2025
29 checks passed
@jrfnl jrfnl deleted the JRF/CS/no-is-null branch March 21, 2025 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog yoast cs/qa

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant