r101606 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r101605‎ | r101606 | r101607 >
Date:09:55, 2 November 2011
Author:ariel
Status:resolved (Comments)
Tags:
Comment:
corrections for fixme in r96486
Modified paths:
  • /trunk/phase3/includes/Export.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Export.php
@@ -871,11 +871,19 @@
872872 protected $filename;
873873
874874 function __construct( $file ) {
875 - $command = setup7zCommand( $file );
 875+ $command = $this->setup7zCommand( $file );
876876 parent::__construct( $command );
877877 $this->filename = $file;
878878 }
879879
 880+ function setup7zCommand( $file ) {
 881+ $command = "7za a -bd -si " . wfEscapeShellArg( $file );
 882+ // Suppress annoying useless crap from p7zip
 883+ // Unfortunately this could suppress real error messages too
 884+ $command .= ' >' . wfGetNull() . ' 2>&1';
 885+ return( $command );
 886+ }
 887+
880888 function closeRenameAndReopen( $newname ) {
881889 $this->closeAndRename( $newname, true );
882890 }
@@ -895,10 +903,7 @@
896904 throw new MWException( __METHOD__ . ": rename of file {$this->filename} to $newname failed\n" );
897905 }
898906 elseif ( $open ) {
899 - $command = "7za a -bd -si " . wfEscapeShellArg( $file );
900 - // Suppress annoying useless crap from p7zip
901 - // Unfortunately this could suppress real error messages too
902 - $command .= ' >' . wfGetNull() . ' 2>&1';
 907+ $command = setup7zCommand( $file );
903908 $this->startCommand( $command );
904909 }
905910 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r101767Address fixme on r101606, should also call from $thisdemon00:41, 3 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r96486get rid of duplication, remove unused function rename(), add documentation as...ariel20:21, 7 September 2011

Comments

#Comment by Platonides (talk | contribs)   23:30, 2 November 2011

You changed a setup7zCommand() to $this->setup7zCommand() but added another setup7zCommand()

Would it be preferable as a static function?

#Comment by ArielGlenn (talk | contribs)   23:33, 2 November 2011

no, it's because I'm dumb, the second one should (probably) also be $this->, but it's 1:30 am. I'll test it tomorrow

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

They were separate issues. First, the wrong call. Second, I proposed making setup7zCommand() as static function.

Status & tagging log