r96816 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96815‎ | r96816 | r96817 >
Date:22:40, 11 September 2011
Author:jeroendedauw
Status:deferred (Comments)
Tags:
Comment:
implemented ratio and expiry settings
Modified paths:
  • /trunk/extensions/Survey/Survey.hooks.php (modified) (history)
  • /trunk/extensions/Survey/includes/SurveyTag.php (modified) (history)
  • /trunk/extensions/Survey/resources/ext.survey.tag.js (modified) (history)

Diff [purge]

Index: trunk/extensions/Survey/Survey.hooks.php
@@ -136,7 +136,7 @@
137137
138138 $surveys = Survey::select(
139139 array(
140 - 'id', 'namespaces'
 140+ 'id', 'namespaces', 'ratio', 'expiry'
141141 ),
142142 array(
143143 'enabled' => 1,
@@ -157,7 +157,9 @@
158158 $GLOBALS['wgOut']->addWikiText( Xml::element(
159159 'survey',
160160 array(
161 - 'id' => $survey->getId()
 161+ 'id' => $survey->getId(),
 162+ 'ratio' => $survey->getField( 'ratio' ),
 163+ 'expiry' => $survey->getField( 'expiry' ),
162164 )
163165 ) );
164166 }
Index: trunk/extensions/Survey/includes/SurveyTag.php
@@ -94,6 +94,8 @@
9595 'cookie' => array(),
9696 'title' => array(),
9797 'require-enabled' => array( 'filter' => FILTER_VALIDATE_INT, 'options' => array( 'min_range' => 0, 'max_range' => 1 ) ),
 98+ 'expiry' => array( 'filter' => FILTER_VALIDATE_INT, 'options' => array( 'min_range' => 0 ) ),
 99+ 'ratio' => array( 'filter' => FILTER_VALIDATE_INT, 'options' => array( 'min_range' => 0, 'max_range' => 100 ) ),
98100 );
99101 }
100102
Index: trunk/extensions/Survey/resources/ext.survey.tag.js
@@ -17,41 +17,53 @@
1818 function shouldShowSurvey( options ) {
1919 var shouldShow = $.cookie( getCookieName( options ) ) != 'done';
2020 survey.log( 'shouldShowSurvey ' + getCookieName( options ) + ': ' + shouldShow.toString() );
21 - return shouldShow;
 21+ return options.cookie || shouldShow;
2222 }
2323
24 - function onSurveyShown( options ) {
25 - var expirySeconds = ( typeof options.expiry !== 'undefined' ) ? options.expiry : 60 * 60 * 24 * 30;
26 - var date = new Date();
27 - date.setTime( date.getTime() + expirySeconds * 1000 );
28 - $.cookie( getCookieName( options ), 'done', { 'expires': date, 'path': '/' } );
 24+ function onSurveyDone( options ) {
 25+ if ( options.cookie ) {
 26+ var date = new Date();
 27+ date.setTime( date.getTime() + options.expiry * 1000 );
 28+ $.cookie( getCookieName( options ), 'done', { 'expires': date, 'path': '/' } );
 29+ survey.log( 'wrote done to cookie ' + getCookieName( options ) );
 30+ }
2931 }
3032
3133 function initTag( $tag ) {
32 - var options = {};
 34+ var ratioAttr = $tag.attr( 'survey-data-ratio' );
 35+ var expiryAttr = $tag.attr( 'survey-data-expiry' );
3336
 37+ var options = {
 38+ 'ratio': typeof ratioAttr === 'undefined' ? 1 : parseFloat( ratioAttr ) / 100,
 39+ 'cookie': $tag.attr( 'survey-data-cookie' ) !== 'no',
 40+ 'expiry': typeof expiryAttr === 'undefined' ? 60 * 60 * 24 * 30 : expiryAttr
 41+ };
 42+
3443 if ( $tag.attr( 'survey-data-id' ) ) {
35 - options['id'] = $tag.attr( 'survey-data-id' );
 44+ options.id = $tag.attr( 'survey-data-id' );
3645 } else if ( $tag.attr( 'survey-data-name' ) ) {
37 - options['name'] = $tag.attr( 'survey-data-name' );
 46+ options.name = $tag.attr( 'survey-data-name' );
3847 }
3948 else {
4049 // TODO
4150 return;
4251 }
4352
44 - if ( $tag.attr( 'survey-data-cookie' ) === 'no' ) {
45 - $tag.mwSurvey( options );
46 - }
47 - else {
 53+ var rand = Math.random();
 54+ survey.log( rand + ' < ' + options.ratio );
 55+
 56+ if ( rand < options.ratio ) {
4857 if ( shouldShowSurvey( options ) ) {
4958 options['onShow'] = function( eventArgs ) {
50 - onSurveyShown( options );
 59+ onSurveyDone( options );
5160 };
5261
5362 $tag.mwSurvey( options );
5463 }
5564 }
 65+ else {
 66+ onSurveyDone( options );
 67+ }
5668 }
5769
5870 $( document ).ready( function() {

Comments

#Comment by Nikerabbit (talk | contribs)   08:58, 12 September 2011

When did you start using this kind of style:

 		}
		else {

?

#Comment by Jeroen De Dauw (talk | contribs)   13:44, 12 September 2011

Long long ago. Over a year. Over a thousand commits. So I'm wondering why people are suddenly asking about it...

#Comment by Nikerabbit (talk | contribs)   13:45, 12 September 2011

Because it's different from MW code style guide? Who else has hasked? :=)

#Comment by Jeroen De Dauw (talk | contribs)   13:49, 12 September 2011

Someone on IRC yesterday, I forgot who. I think the style I'm using fits better w/ the other style rules we have. And I think both styles are acceptable.

#Comment by 😂 (talk | contribs)   13:52, 12 September 2011

It was Jack (ashley). And like I said yesterday: it's an extension....who cares? He can use 8 newlines there if he wants.

#Comment by Siebrand (talk | contribs)   13:58, 12 September 2011

Ehr. That sounds really indifferent, Chad. I think we should care about code conventions, etc. It allows for better code review possibilities, and mentoring people earlier in their committer careers saves time later on. If I - and you too, I'm sure - really wanted to put more time into thinking of reasons to write down here, there surely are many more.

#Comment by 😂 (talk | contribs)   14:00, 12 September 2011

I agree coding conventions are good. But Jeroen isn't new and he knows the coding conventions by now. If he wants to disregard them and it's not a security issue, that's his prerogative. It's just a newline.

#Comment by Siebrand (talk | contribs)   14:07, 12 September 2011

Could we teach stylize.php to fix "}\n\t\t\t[..]else {"? :)

Status & tagging log