[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