Make WordPress Core

Opened 15 years ago

Last modified 2 weeks ago

#13874 assigned enhancement

Add package argument to "_deprecated_function" function

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-patch
Focuses: Cc:

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 15 years ago.
_deprecated_ function enhancements
13874.diff (3.3 KB) - added by ericmann 13 years ago.
Refreshed patch with $package at end of function calls.
13874.2.diff (3.3 KB) - added by wonderboymusic 12 years ago.

Download all attachments as: .zip

Change History (30)

@johnjamesjacoby
15 years ago

_deprecated_ function enhancements

#1 @johnjamesjacoby
15 years ago

This was patched against 3.0-RC3-15241

#2 @demetris
15 years ago

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

#3 follow-up: @nacin
15 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
15 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
15 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
15 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
14 years ago

  • Milestone changed from Awaiting Triage to Future Release

#8 in reply to: ↑ 3 @johnjamesjacoby
13 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
13 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
13 years ago

  • Cc gary@… added

@ericmann
13 years ago

Refreshed patch with $package at end of function calls.

#11 @ericmann
13 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
13 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
12 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
12 years ago

  • Cc nashwan.doaqan@… added

#15 @ryan
12 years ago

  • Milestone changed from 3.6 to Future Release

#16 @wonderboymusic
12 years ago

  • Milestone changed from Future Release to 3.7

westi: "I want to do this in 3.1"

#17 follow-up: @ericlewis
12 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
11 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
11 years ago

  • Cc hotchkissconsulting added

#20 @ericlewis
11 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
11 years ago

  • Cc desrosj@… added

#22 in reply to: ↑ 17 @nacin
11 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
11 years ago

  • Milestone changed from 3.7 to Future Release

#24 @nacin
11 years ago

  • Component changed from Warnings/Notices to Bootstrap/Load

#25 @chriscct7
9 years ago

  • Owner changed from westi to chriscct7
  • Status changed from accepted to assigned

#26 @desrosj
4 weeks ago

  • Keywords needs-refresh added
  • Milestone set to Future Release
  • Owner chriscct7 deleted

I came across this one looking through tickets missing a milestone.

I think this is a neat improvement that could be made. At first glance, I lean towards just adding the argument at the end and leaving the function name the same. It's been 11 additional years at this point and renaming would carry much more risk/consideration.

If folks still want to pursue this, it should be explored in a new ticket.

Removing the ticket owner to make it more clear that no one is actively working on this one.

This ticket was mentioned in PR #8179 on WordPress/wordpress-develop by @sukhendu2002.


2 weeks ago
#27

  • Keywords needs-refresh removed
Note: See TracTickets for help on using tickets.