[wp-trac] [WordPress Trac] #39653: WP_Http_Cookie changes

[WordPress Trac]

#39653: WP_Http_Cookie changes
-------------------------------+----------------------
 Reporter:  sebastian.pisula   |       Owner:  (none)
     Type:  enhancement        |      Status:  closed
 Priority:  normal             |   Milestone:
Component:  HTTP API           |     Version:
 Severity:  normal             |  Resolution:  wontfix
 Keywords:  reporter-feedback  |     Focuses:
-------------------------------+----------------------
Changes (by hellofromTonya):

 * status:  new => closed
 * resolution:   => wontfix
 * milestone:  Awaiting Review =>


Comment:

 Hello @sebastianpisula,

 Thank you for this ticket and patch.

 For context, the `WP_Http_Cookie` class was introduced in [10512] via
 #9049 back in 2009. While yes, Core's coding standards do require
 snake_case naming, caution has been taken to rename previously released
 methods to comply.

 Why?

 Impacts vs Benefits.
 Deprecating the older methods in favor of renamed snake_case methods will
 impact:
 * users and extenders. Existing code in plugins, themes, etc. will throw
 deprecation notices (when `WP_DEBUG` is enabled) which can cause noise,
 increased server log reports, and extra work for triage, support,
 documentation, code changes, etc.
 * contributors and maintainers. Extra code will need to be maintained for
 the long term. Documentation will need to be updated. Extra work and file
 size.

 Do these impacts outweigh the benefits to make the code fully compliant
 with the coding standards?

 That's the question that sometimes is harder to answer. The
 [https://make.wordpress.org/core/handbook/contribute/code-refactoring/
 Code Refactoring guideline] in the handbook helps. Also assessing other
 factors, such as unintended errors or side-effects.

 In this particular class and instance, I'm thinking the impacts outweigh
 the benefits. Likely this is why the decision was made to add the ignore
 in [47632]:
 {{{
 // phpcs:ignore
 WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid
 }}}

 Also noting, there are other classes with older methods that are also
 using the ignore, e.g. `WP_Http` and `WP_HTTP_Response`.

 It's not ideal, as having all of the code fully-compliant would be
 awesome. But when there are impacts, the benefits vs impacts need to be
 considered.

 Closing this ticket as thinking it's a `wontfix`. Happy to reconsider if
 other committers disagree or wish to continue the discussion.

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