For awhile now we've had this pattern for deprecating interfaces: * Mark it deprecated with @deprecated when you deprecate it * In one or two releases add wfDeprecated * A release or two after that or more remove the interface entirely
The rationale of not adding wfDeprecated seams to be so we don't spew warnings about recently deprecated methods to developers. ((Although after a discussion with Krinkle in #mediawiki we had a hard time justifying even that))
The fault of that pattern however is that as releases take time to come out by the time that the next release, or even the release after that, comes by developers have forgotten about the deprecation and forget to add the wfDeprecated for code they changed months ago and don't care about anymore. And the interfaces continue to be used without any notices to help notify people they're still using a deprecated interface which may only be half-functional.
So, I've come up with an idea for a new pattern and committed a change to wfDeprecated.
wfDeprecated now accepts a second arg, the version in which the method was deprecated. You can start use it like so: `wfDeprecated( __METHOD__, '1.19' );` When you use the version arg the notice will include information on when the method was deprecated. Additionally there is a new config setting $wgDeprecationReleaseLimit. If set to a release string alongside $wgDevelopmentWarnings then deprecation notices for releases later than the limit will not be outputted. eg: If you set $wgDeprecationReleaseLimit to "1.18" then `wfDeprecated( __METHOD__, '1.17' );` and `wfDeprecated( __METHOD__, '1.18' );` will generate notices, but `wfDeprecated( __METHOD__, '1.19' );` calls will stay silent. Additionally I've taken branches into account, if you're working in a branch (not a release branch) then please use the pattern: `wfDeprecated( __METHOD__, '1.19-branchname' );` Where 1.19 is likely whatever version trunk is currently at which you are merging from, and branchname is the name of your branch. This pattern will make it easy to search and replace that string when you merge that branch into trunk and will prevent traps like starting a branch in 1.19 and adding wfDeprecated calls with 1.19 as the version, but then actually merging it into trunk when trunk is 1.20 and having to go through each 1.19 tagged wfDeprecated and differentiate between the wfDeprecated calls you added and the wfDeprecated calls that were actually added in 1.19.
Now instead of waiting for a later release I encourage developers to add the wfDeprecated calls right away when you deprecate something. If recent deprecations are too much noise for anyone they can just change the limit to what notices they get.
https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Special:Code/MediaWiki...
On Thu, Sep 15, 2011 at 12:29 PM, Daniel Friesen lists@nadir-seen-fire.com wrote:
For awhile now we've had this pattern for deprecating interfaces:
- Mark it deprecated with @deprecated when you deprecate it
- In one or two releases add wfDeprecated
- A release or two after that or more remove the interface entirely
The rationale of not adding wfDeprecated seams to be so we don't spew warnings about recently deprecated methods to developers. ((Although after a discussion with Krinkle in #mediawiki we had a hard time justifying even that))
The fault of that pattern however is that as releases take time to come out by the time that the next release, or even the release after that, comes by developers have forgotten about the deprecation and forget to add the wfDeprecated for code they changed months ago and don't care about anymore. And the interfaces continue to be used without any notices to help notify people they're still using a deprecated interface which may only be half-functional.
I don't like this. Old interfaces should be either removed or fully supported to the extent that the interface allows. We can't spend our lives going around and changing code to fit some new-fangled interface when you can just make the old interfaces a wrapper around the new ones.
On 11-09-15 05:06 AM, Andrew Garrett wrote:
On Thu, Sep 15, 2011 at 12:29 PM, Daniel Friesen lists@nadir-seen-fire.com wrote:
For awhile now we've had this pattern for deprecating interfaces:
- Mark it deprecated with @deprecated when you deprecate it
- In one or two releases add wfDeprecated
- A release or two after that or more remove the interface entirely
The rationale of not adding wfDeprecated seams to be so we don't spew warnings about recently deprecated methods to developers. ((Although after a discussion with Krinkle in #mediawiki we had a hard time justifying even that))
The fault of that pattern however is that as releases take time to come out by the time that the next release, or even the release after that, comes by developers have forgotten about the deprecation and forget to add the wfDeprecated for code they changed months ago and don't care about anymore. And the interfaces continue to be used without any notices to help notify people they're still using a deprecated interface which may only be half-functional.
I don't like this. Old interfaces should be either removed or fully supported to the extent that the interface allows. We can't spend our lives going around and changing code to fit some new-fangled interface when you can just make the old interfaces a wrapper around the new ones.
Except that's what we do. We create new interfaces replacing interfaces that can't be used to work the way we want them to (ResourceLoader, SkinTemplate's headelement key, methods that are stuck using $wg globals with potential side effects when they should be getting state from their caller, etc...) and we mark the old one deprecated while attempting to leave some temporary partially working compatibility so that extensions have time to update instead of breaking right away. And as for fully removed, that's the ultimate goal of deprecating something. You create a better interface and mark the old one deprecated and try to make it work as much as possible (which since the new interface was created to fix issues in the old one likely means that the interface is only partially capable of working the way it should). Extension maintainers start getting warnings and others help out gradually changing extensions to use the new interface. And in a few releases when you think most of the uses of that old flawed interface are gone you remove it.
Supporting half functional interfaces forever isn't really an option. We'd never have everything going through the resource loader. We'll never be able to create a RequestContext and execute something into it without having to screw with state globals. We'll never be able to have more than one Parser instance because we'd still be supporting the case where some extensions only apply to one parser. ... etc, that's basically saying any project trying to fix the bad parts of our interfaces that stop us from doing some things are pointless.
I'll I'm suggesting is that we start adding wfDeprecated right away instead of a release or so later, and annotating it with the release that it was deprecated. That way people who have a reason to ignore them can ignore them, they can also get an idea of how urgent it is to fix (deprecated in stable vs. deprecated in unreleased trunk), and developers can actually start getting notices earlier that they are using interfaces that they should migrate away from.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name]
Hey,
This is neat.
What about adding another optional argument which allows passing a version variable that the provided version string needs to hold up against? This would allow extensions to also utilize wfDeprecated, which makes sense, since core is not the only place where interfaces are changing :)
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. --
wikitech-l@lists.wikimedia.org