r106883 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r106882‎ | r106883 | r106884 >
Date:22:45, 20 December 2011
Author:jeroendedauw
Status:reverted (Comments)
Tags:
Comment:
partial revert of r106872 after discussion with Brion on #mediawiki irc
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/GlobalFunctions.php
——@@ -3461,7 +3461,7 @@
34623462 function wfDeprecated( $function, $version = false, $component = false ) {
34633463 static $functionsWarned = array();
34643464
3465 - if ( !isset( $functionsWarned[$function] ) ) {
 3465+ if ( !in_array( $function, $GLOBALS['wgDeprecationWhitelist'] ) && !isset( $functionsWarned[$function] ) ) {
34663466 $functionsWarned[$function] = true;
34673467
34683468 if ( $version ) {
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -4090,6 +4090,16 @@
40914091 */
40924092 $wgDeprecationReleaseLimit = '1.17';
40934093
 4094+/**
 4095+ * Function name whitelist for wfDeprecated warnings. You will not be warned
 4096+ * for usage of deprecated functions in this list. This is mainly usefull
 4097+ * for extension developers unable to not use certain deprecated functions
 4098+ * due to backward compatinility reasons.
 4099+ * @since 1.19
 4100+ * @var array
 4101+ */
 4102+$wgDeprecationWhitelist = array();
 4103+
40944104 /** Only record profiling info for pages that took longer than this */
40954105 $wgProfileLimit = 0.0;
40964106

Follow-up revisions

RevisionCommit summaryAuthorDate
r106946follow up to r106883, typo fixesjeroendedauw15:09, 21 December 2011
r109020reverts wgDeprecationWhitelist...hashar08:59, 16 January 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r106872Revert r106456, r106485: system-wide setting seems to be intended to apply on...brion21:52, 20 December 2011

Comments

#Comment by Aaron Schulz (talk | contribs)   02:54, 21 December 2011

"compatinility "?

#Comment by GWicke (talk | contribs)   08:50, 21 December 2011

s/usefull/useful/ in DefaultSettings.php comment

#Comment by Hashar (talk | contribs)   08:54, 21 December 2011

Why mark the function as deprecated in first place? If extensions are forced to use a deprecated function then either:

  1. extension developer need help to migrate its code
  2. the function should not be deprecated

In both case, this whitelist is a useless workaround that will be abused.

#Comment by Aaron Schulz (talk | contribs)   08:55, 21 December 2011

I'd tend to agree.

#Comment by Jeroen De Dauw (talk | contribs)   13:31, 21 December 2011

Having the deprecation notice system is a means for extension developers to find out if they are using any deprecated code that they will need to migrate away from or risk sudden breakage two or three releases of MW later.

> the function should not be deprecated

Sure it should. Not putting in the notice because it might cause warnings for people that do not care about them breaks the whole system, as it does not allow people who care about them to get them. Note the various settings that in practice by default hide most warnings, and need to be explicitly changed to do show them. If there is a lazy extension developer that does not want to care at all, they'll leave dev notices off, not turn them off and put everything in the whitelist, that is just absurd.

> extension developer need help to migrate its code

On the other hand, the whitelist allows developers to ignore deprecation of functions they cannot currently migrate, which in my experience mainly occurs when having wide compat restraints, while still getting everything else. So this argument is as invalid as the other one.

#Comment by Johnduhart (talk | contribs)   04:08, 3 January 2012 
$GLOBALS['wgDeprecationWhitelist']

Why are we using a superglobal for this?

#Comment by Jeroen De Dauw (talk | contribs)   07:34, 6 January 2012

Because this var is only used at one place, so using this approach is clearer and cleaner then placing in a global statement. At least that's my opinion. Either way, it's not like it matters, so let's not fuck ants here shall we?

Status & tagging log