WP_Http_Cookie changes

[sebastian.pisula]

1. The WordPress convention is to use underscores instead of camelCase for function and method change
2. add new param to return array in get_attributes method
3. change parse_url to wp_parse_url

[ocean90]

For 1) see ​https://make.wordpress.org/core/handbook/contribute/code-refactoring/.

Could you explain why you want to have name and value also be returned by get_attributes() and why the use of wp_parse_url() is necessary here?

[hellofromTonya]

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