r111788 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r111787‎ | r111788 | r111789 >
Date:21:22, 17 February 2012
Author:awjrichards
Status:ok (Comments)
Tags:
Comment:
Followup r111777, fixed mixed indentation caused by copy/paste fail
Modified paths:
  • /trunk/extensions/MobileFrontend/MobileFrontend.body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/MobileFrontend.body.php
——@@ -721,10 +721,10 @@
722722 // Is this enough, or should we actually step through the entire
723723 // X-FORWARDED-FOR chain?
724724 if ( isset( $_SERVER['REMOTE_ADDR'] ) ) {
725 - $ip = IP::canonicalize( $_SERVER['REMOTE_ADDR'] );
726 - } else {
727 - $ip = null;
728 - }
 725+ $ip = IP::canonicalize( $_SERVER['REMOTE_ADDR'] );
 726+ } else {
 727+ $ip = null;
 728+ }
729729 if ( wfIsTrustedProxy ( $ip )) {
730730 $wgRequest->response()->header( 'Cache-Control: no-cache, must-revalidate' );
731731 $wgRequest->response()->header( 'Expires: Sat, 26 Jul 1997 05:00:00 GMT' );

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r111777Removed usage of wfGetIP() and now just fetching IP from [REMOTE_ADDR], follo...awjrichards19:28, 17 February 2012

Comments

#Comment by Preilly (talk | contribs)   00:19, 18 February 2012

Just curious why not use a ternary operator?

#Comment by Awjrichards (talk | contribs)   00:22, 18 February 2012

I typically don't really like the ternary operator - particularly in this case, I find the if(){} else{} to just be a lot easier to read.

#Comment by Preilly (talk | contribs)   00:25, 18 February 2012

Hmm, I guess it's a style thing if(){} else{} just looks too verbose to me. Oh, well.... no big deal.

#Comment by Preilly (talk | contribs)   00:23, 18 February 2012

Also, are we supposed to use $_SERVER directly? Or, is it best to use $wgRequest->getHeader?

#Comment by MZMcBride (talk | contribs)   01:47, 18 February 2012

Manual:Coding conventions/PHP#Ternary operator, BTW.

#Comment by Preilly (talk | contribs)   18:23, 18 February 2012

Based on coding conventions there is no issue with suggesting the use of a ternary operator in this case, BTW.

#Comment by MZMcBride (talk | contribs)   18:25, 18 February 2012

You're so saucy. <3

Status & tagging log