r68606 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r68605‎ | r68606 | r68607 >
Date:13:25, 26 June 2010
Author:mgrabovsky
Status:reverted (Comments)
Tags:
Comment:
(bug 18891) Deprecated Article::insertNewArticle still used in core
Modified paths:
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/maintenance/addwiki.php (modified) (history)
  • /trunk/phase3/maintenance/parserTests.inc (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/parserTests.inc
@@ -1040,7 +1040,8 @@
10411041 }
10421042
10431043 $art = new Article( $title );
1044 - $art->insertNewArticle( $text, '', false, false );
 1044+ $art->doEdit( $text, '', EDIT_NEW | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY,
 1045+ false, null, false, false, '', true );
10451046
10461047 $this->teardownGlobals();
10471048 }
Index: trunk/phase3/maintenance/addwiki.php
@@ -130,7 +130,8 @@
131131 $wgArticle = new Article( $wgTitle );
132132 $ucsite = ucfirst( $site );
133133
134 - $wgArticle->insertNewArticle( $this->getFirstArticle( $ucsite, $name ), '', false, false );
 134+ $wgArticle->doEdit( $this->getFirstArticle( $ucsite, $name ), '', EDIT_NEW | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY,
 135+ false, null, false, false, '', true );
135136
136137 $this->output( "Adding to dblists\n" );
137138
Index: trunk/phase3/docs/hooks.txt
@@ -653,7 +653,7 @@
654654 $editPage: EditPage object
655655
656656 'EditPage::attemptSave': called before an article is
657 -saved, that is before insertNewArticle() is called
 657+saved, that is before Article::doEdit() is called
658658 $editpage_Obj: the current EditPage object
659659
660660 'EditPage::importFormData': allow extensions to read additional data
Index: trunk/phase3/includes/Article.php
@@ -1965,6 +1965,7 @@
19661966 * it. Nevertheless, use Article::doEdit() instead.
19671967 */
19681968 function insertNewArticle( $text, $summary, $isminor, $watchthis, $suppressRC = false, $comment = false, $bot = false ) {
 1969+ wfDeprecated( __METHOD__ );
19691970 $flags = EDIT_NEW | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY |
19701971 ( $isminor ? EDIT_MINOR : 0 ) |
19711972 ( $suppressRC ? EDIT_SUPPRESS_RC : 0 ) |
Index: trunk/phase3/includes/EditPage.php
@@ -905,10 +905,11 @@
906906
907907 $isComment = ( $this->section == 'new' );
908908
909 - # FIXME: paste contents from Article::insertNewArticle here and
910 - # actually handle errors it may return
911 - $this->mArticle->insertNewArticle( $this->textbox1, $this->summary,
912 - $this->minoredit, $this->watchthis, false, $isComment, $bot );
 909+ $flags = EDIT_NEW | EDIT_DEFER_UPDATES | EDIT_AUTOSUMMARY |
 910+ ( $this->minoredit ? EDIT_MINOR : 0 ) |
 911+ ( $bot ? EDIT_FORCE_BOT : 0 );
 912+ $this->mArticle->doEdit( $this->textbox1, $this->summary, $flags,
 913+ false, null, $this->watchthis, $isComment, '', true );
913914
914915 wfProfileOut( __METHOD__ );
915916 return self::AS_SUCCESS_NEW_ARTICLE;
Index: trunk/phase3/includes/Title.php
@@ -2345,10 +2345,6 @@
23462346 * This clears some fields in this object, and clears any associated
23472347 * keys in the "bad links" section of the link cache.
23482348 *
2349 - * - This is called from Article::insertNewArticle() to allow
2350 - * loading of the new page_id. It's also called from
2351 - * Article::doDeleteArticle()
2352 - *
23532349 * @param $newid \type{\int} the new Article ID
23542350 */
23552351 public function resetArticleID( $newid ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r68608Follow-up r68606, per comments:...mgrabovsky19:52, 26 June 2010
r68609Follow-up r68606, per comments:...mgrabovsky19:52, 26 June 2010
r70760* Revert r66878, completely misses the point of factoring out doEdit() in the...tstarling08:33, 9 August 2010

Comments

#Comment by Bryan (talk | contribs)   13:51, 26 June 2010

Now that the proper Article::doEdit is used, its return value should be checked to see whether the edit was actually succesful.

#Comment by Matěj Grabovský (talk | contribs)   19:56, 26 June 2010

Should be fixed in r68609.

#Comment by 😂 (talk | contribs)   15:01, 26 June 2010

Also need to remove the couple of uses still left in /trunk/extensions before adding wfDeprecated(). I count 3.

#Comment by Matěj Grabovský (talk | contribs)   19:55, 26 June 2010

Fixed in r68608.

#Comment by Nikerabbit (talk | contribs)   19:59, 26 June 2010

While at it, can we decide now when to remove that function and document that?

#Comment by Bryan (talk | contribs)   20:21, 26 June 2010

Never, in my opinion. I don't think there

#Comment by Bryan (talk | contribs)   20:21, 26 June 2010

... is sufficient reason to break b/c.

#Comment by Nikerabbit (talk | contribs)   20:51, 26 June 2010

We should have agreed policy on this. I don't think we have resources to keep all old interfaces working (like Windows), but we could do it like X, where old stuff is removed when they bitrot enough to be broken. The bad thing in that is that developers can just ignore the warnings until it will suddenly break some day. If we decide before hand they can at least know when it happens.

Status & tagging log