[wp-trac] [WordPress Trac] #54323: Too few arguments for function (closure)

WordPress Trac noreply at wordpress.org
Wed Oct 27 23:30:10 UTC 2021


#54323: Too few arguments for function (closure)
-------------------------------------------------+-------------------------
 Reporter:  aristath                             |       Owner:
                                                 |  SergeyBiryukov
     Type:  defect (bug)                         |      Status:  reopened
 Priority:  normal                               |   Milestone:  5.8.2
Component:  Script Loader                        |     Version:
 Severity:  major                                |  Resolution:
 Keywords:  has-patch needs-unit-tests fixed-    |     Focuses:  coding-
  major                                          |  standards
-------------------------------------------------+-------------------------

Comment (by jrf):

 Replying to [comment:8 SergeyBiryukov]:
 > Replying to [comment:7 jrf]:
 > > While the above is true, I still think it should be changed as it may
 give new(er) contributors the impression that closures as hook callbacks
 are acceptable. Consistency is key in these things and having a rule like
 this applied inconsistently only provides confusion and leads to repeated
 discussions about it (''why is it allowed there, but not here ?'').
 >
 > That makes perfect sense to me :) I agree, let's change this to a proper
 callback. Worth noting that another instance of a closure as a hook
 callback was also committed recently in [51902] and would need a similar
 change.

 Yes please!

 If I remember correctly, there is an open WPCS ticket to create a sniff to
 detect this (closures used as hook callbacks). When I find some time, I
 might just need to make sure that sniff gets written and merged to be
 included in the next WPCS release. (but 😫, where do I find the time... ?)

 >
 > > Ouch. That's just painful. Old code not having tests I can understand,
 we're playing catch-up. But greenfields new code being introduced without
 tests or with insufficient tests is something I'd really like to
 eradicate.
 >
 > Also agree here. But how do we resolve this? Code like this is merged
 from Gutenberg. I find the current pace of Gutenberg development a bit too
 fast to keep up or wrap my head around it enough to write meaningful
 tests.

 If I'm honest, this is not something for you or me to solve, but something
 which needs to be addressed in the Gutenberg project. We basically need a
 firm stance there that no code is allowed to be merged into Gutenberg (or
 any other new feature sub-projects - fonts API, plugin dependencies API,
 I'm looking at you!) without tests covering that code.
 This policy should apply to both JS code, as well as PHP code.
 On top of that, we need a policy for Core that lack of tests/insufficient
 test code coverage for new code will be considered a non-negotiable
 blocker for merges into Core.

 I suppose we need to put this to ''the powers that be'' to get a decision,
 but the way things are, we're just introducing more and more bugs and
 technical debt instead of solving anything.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/54323#comment:9>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list