WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#16757 closed enhancement (fixed)

Removal of wpdb::supports_collation()

Reported by: hakre Owned by: nacin
Milestone: 3.5 Priority: lowest
Severity: trivial Version: 3.1
Component: Database Keywords: has-patch
Focuses: Cc:

Description

In 2.5 wpdb::supports_collation() had been introduced and used in a single place inside core, that was wp-admin/includes/schema.php r6199. In r8740 the use has been switched over to wpdb::has_cap() which was 2.7 then.

As usual there might be third party code that is still making use of this function but since it can be ported backwards compatible to wpdb::has_cap( 'collation' ) for wp 2.7 and above, and it is documented to be used only at one place (see the phpdoc of the function) and it was actually used in only one place over the years and already has been subject to improvement, I suggest to just remove it.

Third party code will do a straight fatal error that makes it easy to locate where to apply a change:

$wpdb->supports_collation() -> $wpdb->has_cap( 'collation' )

Attachments (4)

16757.patch (610 bytes) - added by hakre 4 years ago.
16757.2.patch (539 bytes) - added by hakre 4 years ago.
deprecated function call
16757.3.patch (454 bytes) - added by hakre 4 years ago.
16757.diff (618 bytes) - added by pento 3 years ago.

Download all attachments as: .zip

Change History (16)

@hakre4 years ago

comment:1 @hakre4 years ago

  • Keywords dev-feedback added

It might make sense to deprecate the function first but that might be considered to be bloat in this case. Feedback appreceated.

comment:2 @hakre4 years ago

Related: #16764

comment:3 follow-ups: @dd324 years ago

What's this got to do with PHP5?

And other than breaking stuff, Why remove it? (_depecated_function() perhaps, but remove, no?)

comment:4 @westi4 years ago

  • Component changed from General to Database
  • Keywords needs-patch added; has-patch dev-feedback removed
  • Priority changed from normal to lowest
  • Severity changed from normal to trivial
  • Summary changed from PHP5-port - Removal of wpdb::supports_collation() to Removal of wpdb::supports_collation()

I don't see any point in removing this deprecating maybe yes.

Removing PHP5 for subject because this is nothing to do with PHP5

comment:5 in reply to: ↑ 3 @hakre4 years ago

Replying to dd32:

What's this got to do with PHP5?

While refactoring the whole wpdb class I ran over this function as a probably removal candidate from the class's interface. The related ticket is #16764, an interface has been written down in 16764.patch

comment:6 in reply to: ↑ 3 @hakre4 years ago

Replying to dd32:

And other than breaking stuff, Why remove it? (_depecated_function() perhaps, but remove, no?)

As far as core is concerned this doesn't break anything. The proposed usage for third party code is to use alternative has_cap function anyway so - as written - chance is very likely that this removal won't break anything.

Additionally as written, deprecation has been concerned if it makes sense. You have not provided much information to make a dedicated decision which of the two alternatives should be preferred.

If you can shed some more light into this issue this would be really appreciated.

Writing a patch that contains the call to deprecated function is rather trivial. The actual decision is more important in my eyes.

@hakre4 years ago

deprecated function call

comment:7 @hakre4 years ago

  • Keywords has-patch added; needs-patch removed

comment:8 follow-up: @westi4 years ago

  • Keywords needs-refresh added

Please update the patch to adhere to the Coding style and to include the arguments directly in the function call rather than defining variables for single usage.

@hakre4 years ago

comment:9 in reply to: ↑ 8 @hakre4 years ago

  • Keywords needs-refresh removed

Replying to westi:

Please update the patch to adhere to the Coding style and to include the arguments directly in the function call rather than defining variables for single usage.

Updated the patch, please review.

@pento3 years ago

comment:10 @pento3 years ago

Refreshed patch. Let's deprecate this thing.

comment:11 @nacin3 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [21470]:

Deprecate wpdb::supports_collation() in favor of wpdb::has_cap().

props hakre, pento. fixes #16757.

comment:12 @nacin3 years ago

  • Milestone changed from Awaiting Review to 3.5
Note: See TracTickets for help on using tickets.