[wp-trac] [WordPress Trac] #43583: Introduce new PHP cross-version compat function `is_countable()`
WordPress Trac
noreply at wordpress.org
Wed May 9 10:09:07 UTC 2018
#43583: Introduce new PHP cross-version compat function `is_countable()`
-------------------------------------------------+-------------------------
Reporter: jrf | Owner:
| SergeyBiryukov
Type: enhancement | Status: reopened
Priority: normal | Milestone: 4.9.6
Component: General | Version:
Severity: normal | Resolution:
Keywords: has-patch has-unit-tests needs-dev- | Focuses:
note commit |
-------------------------------------------------+-------------------------
Comment (by jrf):
@desrosj Thanks for the follow-up patch. Looking good.
A couple of small remarks:
> Fix alignment of new conditional statements.
It looks like the follow on conditions have a unintentional space
precision alignment (which should be removed). Alignment should be three
tabs and it looks like the current patch has three tabs + one space.
> Added parameter inline documentation for
test_is_countable_functionality().
These should use `@param` instead of `@type`. `@type` is only used when
specifying the details for individual keys in an `array` return.
Otherwise 👍.
> Split out SimpleXMLElement and ResourceBundle tests into their own test
methods so that they can be marked as skipped if their respective
extensions are not loaded.
👍
May I suggest moving these two new test functions to below the data
provider for the `test_is_countable_functionality()` function ? That way
the test + data provider stay together, making them easier to find/connect
for other devs looking at the tests.
> Change assertEquals() to assertSame()
👍
--
Ticket URL: <https://core.trac.wordpress.org/ticket/43583#comment:22>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list