WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 5 months ago

#13874 assigned enhancement

Add package argument to "_deprecated_function" function

Reported by: johnjamesjacoby Owned by: chriscct7
Milestone: Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-patch
Focuses: Cc:
PR Number:

Description

Got to thinking it would be nice if plugins could use the _deprecated_ API by passing a "$package" argument to separate themselves from WP core, and so this patch was born.

Attachments (3)

_deprecated_package.patch (73.6 KB) - added by johnjamesjacoby 9 years ago.
_deprecated_ function enhancements
13874.diff (3.3 KB) - added by ericmann 8 years ago.
Refreshed patch with $package at end of function calls.
13874.2.diff (3.3 KB) - added by wonderboymusic 7 years ago.

Download all attachments as: .zip

Change History (28)

@johnjamesjacoby
9 years ago

_deprecated_ function enhancements

#1 @johnjamesjacoby
9 years ago

This was patched against 3.0-RC3-15241

#2 @demetris
9 years ago

Why not put the new argument at the end and give it a default value of wp-core?

#3 follow-up: @nacin
9 years ago

We would be breaking compatibility with plugins that currently use this by rearranging arguments. $package = 'core' at the end seems sensible to me.

#4 @johnjamesjacoby
9 years ago

What plugins currently use this? If they're deprecating their own functions this way, they're breaking deprecated functions for core, since WP only knows to deprecate its own functions.

I also figured tacking it to the beginning made sense from a code readability perspective; file->package->version->function

..but, I'd settle for whatever is easiest.

#5 @demetris
9 years ago

My concern is not so much backwards compatibility as readability.

The way you added it is more logical and makes it easier and more readable for the few external scripts that will use the function; core code, on the other hand, is more compact and more readable when it uses the default value of an argument that comes after all arguments that it has to define.

#6 @westi
9 years ago

  • Milestone set to 3.1
  • Owner set to westi
  • Status changed from new to accepted

I want to do this in 3.1

#7 @nacin
9 years ago

  • Milestone changed from Awaiting Triage to Future Release

#8 in reply to: ↑ 3 @johnjamesjacoby
8 years ago

  • Keywords needs-refresh added

Replying to nacin:

We would be breaking compatibility with plugins that currently use this by rearranging arguments. $package = 'core' at the end seems sensible to me.

After further review, I agree. Recent Twitter conversations have brought this back up; it'd be great to get a refreshed patch on this.

#9 @ericmann
8 years ago

+1 To this idea, though I agree that new arguments should be placed at the end. Whether or not people should have been using this functionality in plugins before doesn't really matter. Fact is, people have used it out of the interest of being helpful to other devs. We should make an effort to not break things when modifying functionality.

#10 @GaryJ
8 years ago

  • Cc gary@… added

@ericmann
8 years ago

Refreshed patch with $package at end of function calls.

#11 @ericmann
8 years ago

  • Keywords needs-refresh removed

As I started walking through the code to refresh the patch, I realized that placing $package at the end of the function calls, along with setting a default value of "core," made this a relatively small patch to apply. There's no real need to walk back through every instance of the _deprecated_function()/_deprecated_file()/_deprecated_argument() calls since they'll all inherit the right value for $package anyway.

But this should open things up for plugin and theme devs to specify their own deprecated functions in the future.

#12 @strider72
8 years ago

Just a note to suggest that while this is a convenience, it isn't technically necessary, as a backtrace can readily identify the file in which the deprecated function was called.

My "Log Deprecated Calls" plugin does this: http://striderweb.com/nerdaphernalia/features/wp-log-deprecated-calls/

I have nothing against convenience, of course. ;-)

#13 @wonderboymusic
7 years ago

  • Milestone changed from Future Release to 3.6

westi: "I want to do this in 3.1" - my patch is identical to the one before it, just applies cleaner to trunk

#14 @alex-ye
6 years ago

  • Cc nashwan.doaqan@… added

#15 @ryan
6 years ago

  • Milestone changed from 3.6 to Future Release

#16 @wonderboymusic
6 years ago

  • Milestone changed from Future Release to 3.7

westi: "I want to do this in 3.1"

#17 follow-up: @ericlewis
6 years ago

Modifying _deprecated_function and _deprecated_file to allow for more integration with third-party code is great. However, these functions remain prefixed with underscores to denote them as private, as well as in the Docblock above. Deprecating these deprecated functions in lieu of un-prefixed ones may be overkill and quite the meta commit, but we should at least change the Dockblock.

#18 @hotchkissconsulting
6 years ago

Why not add wp_deprecated_function, wp_deprecated_argument and wp_deprecated_file, which are just passthroughs to the private functions, then eventually deprecate _deprecated_function, etc?

#19 @hotchkissconsulting
6 years ago

  • Cc hotchkissconsulting added

#20 @ericlewis
6 years ago

Related: #9927 and #24164

I'm coming across more and more situations where a function's name is misleading, and there's a suggestion to rename it; a top-down decision on what to do in these situations would be good.

#21 @desrosj
6 years ago

  • Cc desrosj@… added

#22 in reply to: ↑ 17 @nacin
6 years ago

Replying to ericlewis:

Modifying _deprecated_function and _deprecated_file to allow for more integration with third-party code is great. However, these functions remain prefixed with underscores to denote them as private, as well as in the Docblock above. Deprecating these deprecated functions in lieu of un-prefixed ones may be overkill and quite the meta commit, but we should at least change the Dockblock.

I don't feel strongly either way, but if we're going to add package support to this, new functions (with the package argument being first) might not be a bad idea.

I'm coming across more and more situations where a function's name is misleading, and there's a suggestion to rename it; a top-down decision on what to do in these situations would be good.

Usually we err on not renaming functions for the sake of renaming them. We would rather do things like overhaul their internals or arguments in the process, otherwise we're just using up the existing namespace (and annoying developers by deprecating potentially commonly used functions just for a rename).

#23 @nacin
6 years ago

  • Milestone changed from 3.7 to Future Release

#24 @nacin
6 years ago

  • Component changed from Warnings/Notices to Bootstrap/Load

#25 @chriscct7
4 years ago

  • Owner changed from westi to chriscct7
  • Status changed from accepted to assigned
Note: See TracTickets for help on using tickets.