r103817 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103816‎ | r103817 | r103818 >
Date:16:13, 21 November 2011
Author:johnduhart
Status:resolved (Comments)
Tags:parser 
Comment:
Bug 29524 - Rename RequestContext::getLang to getLanguage

I'll be amazed if this doens't break any tests.
Modified paths:
  • /trunk/phase3/includes/Action.php (modified) (history)
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/CategoryViewer.php (modified) (history)
  • /trunk/phase3/includes/ChangesList.php (modified) (history)
  • /trunk/phase3/includes/Message.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/Pager.php (modified) (history)
  • /trunk/phase3/includes/Preferences.php (modified) (history)
  • /trunk/phase3/includes/QueryPage.php (modified) (history)
  • /trunk/phase3/includes/RevisionList.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)
  • /trunk/phase3/includes/SpecialPageFactory.php (modified) (history)
  • /trunk/phase3/includes/StubObject.php (modified) (history)
  • /trunk/phase3/includes/actions/CreditsAction.php (modified) (history)
  • /trunk/phase3/includes/actions/HistoryAction.php (modified) (history)
  • /trunk/phase3/includes/actions/InfoAction.php (modified) (history)
  • /trunk/phase3/includes/actions/RevertAction.php (modified) (history)
  • /trunk/phase3/includes/cache/HTMLFileCache.php (modified) (history)
  • /trunk/phase3/includes/context/ContextSource.php (modified) (history)
  • /trunk/phase3/includes/context/DerivativeContext.php (modified) (history)
  • /trunk/phase3/includes/context/IContextSource.php (modified) (history)
  • /trunk/phase3/includes/context/RequestContext.php (modified) (history)
  • /trunk/phase3/includes/diff/DifferenceEngine.php (modified) (history)
  • /trunk/phase3/includes/logging/LogFormatter.php (modified) (history)
  • /trunk/phase3/includes/parser/ParserOptions.php (modified) (history)
  • /trunk/phase3/includes/revisiondelete/RevisionDelete.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialActiveusers.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialAllmessages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialAllpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialAncientpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialBlock.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialBlockList.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialBrokenRedirects.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialCategories.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialChangePassword.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialConfirmemail.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialContributions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialDeletedContributions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialDisambiguations.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialDoubleRedirects.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialEditWatchlist.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialFewestrevisions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialFileDuplicateSearch.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialImport.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialLinkSearch.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialListfiles.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialListgrouprights.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialListredirects.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialListusers.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMIMEsearch.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMergeHistory.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMostcategories.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMostlinked.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMostlinkedcategories.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMostlinkedtemplates.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialMovepage.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialNewimages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialNewpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialPopularpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialProtectedpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialProtectedtitles.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialRecentchanges.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialRevisiondelete.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialSearch.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialShortpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialStatistics.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialTags.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUndelete.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUnusedtemplates.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUnwatchedpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUpload.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserlogin.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserrights.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialVersion.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialWantedcategories.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialWatchlist.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialWhatlinkshere.php (modified) (history)
  • /trunk/phase3/skins/CologneBlue.php (modified) (history)
  • /trunk/phase3/skins/Standard.php (modified) (history)
  • /trunk/phase3/tests/parser/parserTest.inc (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/suites/UploadFromUrlTestSuite.php (modified) (history)

Follow-up revisions

RevisionCommit summaryAuthorDate
r103911Changes from r103817johnduhart16:08, 22 November 2011
r104011Followup r103817, backing out ContextSource changes to SpecialPage and Actionjohnduhart09:53, 23 November 2011
r104015Followup r103817, updating setLang usagesjohnduhart10:28, 23 November 2011
r105205Add missing @since tags for getLanguage and deal with this sanitizeLangCode f...dantman18:56, 5 December 2011

Comments

#Comment by Johnduhart (talk | contribs)   16:15, 21 November 2011

Oh, err. This commit also makes Action and SpecialPage context sources.

#Comment by IAlex (talk | contribs)   17:26, 21 November 2011

The reason why SpecialPage was not extending ContextSource is that SpecialPage::getTitle() differs from ContextSource::getTitle() since the former returns a Title object without subpage (when the argument is not passed) and the latter returns the full Title with subpage.

#Comment by Johnduhart (talk | contribs)   22:32, 21 November 2011

Okay so why can't we just override it?

#Comment by Catrope (talk | contribs)   20:53, 22 November 2011

Because there's all sorts of code out there that relies on the current semantics of SpecialPage::getTitle(), as well as code that relies on the semantics of RequestContext::getTitle(). Those semantics conflict with each other, so they cannot be provided by the same function.

#Comment by Brion VIBBER (talk | contribs)   23:46, 22 November 2011

Marking fixme for the transformation of things into context sources that have conflicts. Any objection to a revert?

#Comment by Dantman (talk | contribs)   19:07, 5 December 2011

Frankly I'd revert on the fact alone that we have an interface declaration that includes a required method that says `@deprecated 1.19 Use getLanguage instead` in it, and the only reason for making the name change causing duplicate methods in the interface was stylistic changes to the name, absolutely no functional changes to the code.

#Comment by Platonides (talk | contribs)   23:53, 22 December 2011

We finally shipped 1.18 without this, so revert it and stick to the old name.

#Comment by Tim Starling (talk | contribs)   05:01, 2 January 2012

Note that RevisionListBase was also made into a ContextSource.

The method change is OK since there is a b/c alias. The changes to SpecialPage and Action were reverted in r104011. The RevisionListBase change is OK. Marking resolved.

Status & tagging log