MediaWiki r66200 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r66199‎ | r66200 (on ViewVC)‎ | r66201 >
Date:16:24, 11 May 2010
Author:mgrabovsky
Status:ok (Comments)
Tags:
Comment:
(bug 23426) The {{REVISIONMONTH}} variable is now zero-padded and added new variable {{REVISIONMONTH1}} when unpadded version is needed.
Modified paths:

Diff [purge]

Index: trunk/phase3/includes/parser/Parser.php
===================================================================
--- trunk/phase3/includes/parser/Parser.php	(revision 66199)
+++ trunk/phase3/includes/parser/Parser.php	(revision 66200)
@@ -2528,6 +2528,13 @@
 				# *after* a revision ID has been assigned. This is for null edits.
 				$this->mOutput->setFlag( 'vary-revision' );
 				wfDebug( __METHOD__ . ": {{REVISIONMONTH}} used, setting vary-revision...\n" );
+				$value = substr( $this->getRevisionTimestamp(), 4, 2 );
+				break;
+			case 'revisionmonth1':
+				# Let the edit saving system know we should parse the page
+				# *after* a revision ID has been assigned. This is for null edits.
+				$this->mOutput->setFlag( 'vary-revision' );
+				wfDebug( __METHOD__ . ": {{REVISIONMONTH1}} used, setting vary-revision...\n" );
 				$value = intval( substr( $this->getRevisionTimestamp(), 4, 2 ) );
 				break;
 			case 'revisionyear':
Index: trunk/phase3/includes/MagicWord.php
===================================================================
--- trunk/phase3/includes/MagicWord.php	(revision 66199)
+++ trunk/phase3/includes/MagicWord.php	(revision 66200)
@@ -79,6 +79,7 @@
 		'revisionday',
 		'revisionday2',
 		'revisionmonth',
+		'revisionmonth1',
 		'revisionyear',
 		'revisiontimestamp',
 		'revisionuser',
Index: trunk/phase3/languages/messages/MessagesEn.php
===================================================================
--- trunk/phase3/languages/messages/MessagesEn.php	(revision 66199)
+++ trunk/phase3/languages/messages/MessagesEn.php	(revision 66200)
@@ -311,6 +311,7 @@
 	'revisionday'            => array( 1,    'REVISIONDAY'            ),
 	'revisionday2'           => array( 1,    'REVISIONDAY2'           ),
 	'revisionmonth'          => array( 1,    'REVISIONMONTH'          ),
+	'revisionmonth1'         => array( 1,    'REVISIONMONTH1'         ),
 	'revisionyear'           => array( 1,    'REVISIONYEAR'           ),
 	'revisiontimestamp'      => array( 1,    'REVISIONTIMESTAMP'      ),
 	'revisionuser'           => array( 1,    'REVISIONUSER'           ),
Index: trunk/phase3/RELEASE-NOTES
===================================================================
--- trunk/phase3/RELEASE-NOTES	(revision 66199)
+++ trunk/phase3/RELEASE-NOTES	(revision 66200)
@@ -160,6 +160,8 @@
 * (bug 23422) mp3 files can now be moved
 * (bug 23448) MediaWiki:Summary-preview is now displayed instead of
   MediaWiki:Subject-preview when previewing summary
+* (bug 23426) The {{REVISIONMONTH}} variable is now zero-padded and added new
+  variable {{REVISIONMONTH1}} when unpadded version is needed.
 
 === API changes in 1.17 ===
 * (bug 22738) Allow filtering by action type on query=logevent

Follow-up revisions

Rev.Commit summaryAuthorDate
r81414Test suite for revisions and date related magic variables....hashar21:04, 2 February 2011

Comments

#Comment by Raymond (Talk | contribs)   18:29, 11 May 2010

Wouldn't it be more consistent to name them:

  • {{REVISIONMONTH}} = unpadded (unchanged behaviour)
  • {{REVISIONMONTH2}} = zero-padded

like we already have:

  • {{REVISIONDAY}} = 0 = unpadded
  • {{REVISIONDAY2}} = = zero-padded
#Comment by Matěj Grabovský (Talk | contribs)   18:43, 11 May 2010

Actually the current consistency is that month is zero-padded by default and day is unpadded by default.

See

  • CURRENTMONTH, LOCALMONTH – zero-padded
  • CURRENTMONTH1, LOCALMONTH1 – unpadded

and

  • CURRENTDAY, LOCALDAY – unpadded
  • CURRENTDAY2, LOCALDAY2 – zero-padded

If we do the proposed change with REVISIONMONTH we should also do this CURRENT/LOCALMONTH.

#Comment by Hashar (Talk | contribs)   22:13, 7 December 2010

Subscribing to this revision notification. I am writing unit tests for this.

#Comment by Reedy (Talk | contribs)   03:15, 6 January 2011

Ping

#Comment by Hashar (Talk | contribs)   07:38, 6 January 2011

I am still working on the tests although I did not made any progress with the Xmas break ;-b

#Comment by Hashar (Talk | contribs)   21:05, 2 February 2011

test suite with r81414. Still need to decide what we want to do.

#Comment by ^demon (Talk | contribs)   22:59, 15 April 2011

Can we please just decide something? This has been sitting here for almost a year now. One way or the other, pick one.

Ping Raymond, Matěj, Hashar, please.

#Comment by Bryan (Talk | contribs)   08:13, 16 April 2011

It has been deployed like this already, so changing it would break stuff. I'd say we'll keep it as it is and mark this ok.

#Comment by Hashar (Talk | contribs)   18:16, 1 May 2011

I personally don't care. I was just raising the possible inconstancy enlighten by r81414 tests:

[x] Currentday is un padded
[x] Currentdaytwo is zero padded
[x] Localday is un padded
[x] Localdaytwo is zero padded
[x] Currentmonth is zero padded
[x] Currentmonthone is un padded
[x] Localmonth is zero padded
[x] Localmonthone is un padded
[x] Revisionday is un padded
[x] Revisiondaytwo is zero padded
[x] Revisionmonth is zero padded
[x] Revisionmonthone is un padded

It should be revisionmonttwo to be consistant with our other uses.

#Comment by Bryan (Talk | contribs)   11:58, 6 May 2011

Has already been deployed and almost released. Let's not break stuff again, so marking this as ok.

Status & tagging log

Personal tools
Namespaces
Variants
Views
Actions
Site
Support
Download
Development
Communication
Toolbox