Skip to content

UnslashingFunctionsHelper::is_unslashing_function(): add tests#2715

Open
rodrigoprimo wants to merge 2 commits intoWordPress:developfrom
rodrigoprimo:unslashing-functions-tests
Open

UnslashingFunctionsHelper::is_unslashing_function(): add tests#2715
rodrigoprimo wants to merge 2 commits intoWordPress:developfrom
rodrigoprimo:unslashing-functions-tests

Conversation

@rodrigoprimo
Copy link
Copy Markdown
Collaborator

Description

In preparation for supporting PHPCS 4.0, this PR adds unit tests for the UnslashingFunctionsHelper::is_unslashing_function() method.

Suggested changelog entry

N/A

Copy link
Copy Markdown
Member

@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.

LGTM.

I presume you have verified that there are no stray @covers tags for this function in pre-existing tests ?

@rodrigoprimo
Copy link
Copy Markdown
Collaborator Author

rodrigoprimo commented Apr 2, 2026

Thanks for your review, @jrfnl!

I presume you have verified that there are no stray @covers tags for this function in pre-existing tests ?

Yes, I have, and there are no stray @covers tags for this function in the pre-existing tests.

Remove trailing `()` from the `@covers` tag and reorder `@see` to come
before `@return`.
@rodrigoprimo
Copy link
Copy Markdown
Collaborator Author

I pushed a new commit to this PR addressing two minor issues that I noticed while working on a similar PR:

  • Trailing () in the method name in the @covers tag.
  • @see and @return tags in the wrong order.

@jrfnl
Copy link
Copy Markdown
Member

jrfnl commented Apr 2, 2026

@rodrigoprimo Yes, I noticed what you mention in the second bullet (also in #2713) and the way it is now is definitely preferred, but it was not a blocker. Good catch on the first. I missed that.

@jrfnl
Copy link
Copy Markdown
Member

jrfnl commented Apr 2, 2026

For whomever does the second review: please squash-merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants