r101767 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101766‎ | r101767 | r101768 >
Date:00:41, 3 November 2011
Author:demon
Status:ok (Comments)
Tags:
Comment:
Address fixme on r101606, should also call from $this
Modified paths:
  • /trunk/phase3/includes/Export.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Export.php
——@@ -896,7 +896,7 @@
897897 proc_close( $this->procOpenResource );
898898 $this->renameOrException( $newname );
899899 if ( $open ) {
900 - $command = setup7zCommand( $file );
 900+ $command = $this->setup7zCommand( $file );
901901 $this->startCommand( $command );
902902 }
903903 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r103054Bringing in Export.php from the copy at r101767reedy22:22, 14 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r101606corrections for fixme in r96486ariel09:55, 2 November 2011

Comments

#Comment by Platonides (talk | contribs)   16:39, 3 November 2011

Yes, but what I was proposing was to make it a static function, not using a static call.

#Comment by 😂 (talk | contribs)   16:45, 3 November 2011

It's not used outside of this class, so I'm not sure why it needs to be static?

#Comment by Platonides (talk | contribs)   22:46, 3 November 2011

Why would it need to have the $this pointer? (reversing your query)

As it isn't using the class for anything, seems cleaner for me to make it static.

#Comment by 😂 (talk | contribs)   22:48, 3 November 2011

Six of one, half a dozen of another. We can change it to static if it's bothering you...I don't particularly care :)

#Comment by ArielGlenn (talk | contribs)   17:03, 14 November 2011

My rationale for not static is that I can't see a use case for it outside of the specific class. So at least for now let's leave it. Also thanks demon for getting here before me, got swamped.

Status & tagging log