[MediaWiki-commits] [Gerrit] Fix CentralNotice Underallocation - change (mediawiki...CentralNotice)

2013-02-26 Thread Mwalker (Code Review) 
Mwalker has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/50931


Change subject: Fix CentralNotice Underallocation
..

Fix CentralNotice Underallocation

CentralNotice should have been giving at least 1 slot to each banner.
In the case where low weight banners were at the end of the banner
list they could be missed during the reallocation step.

This has been solved with sorting, explicit minimum, and taking
care of any potential overallocation.

Change-Id: I2214ed49ede39718bc22cb69e9662ad344beacc6
---
M includes/BannerChooser.php
1 file changed, 16 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CentralNotice 
refs/changes/31/50931/1

diff --git a/includes/BannerChooser.php b/includes/BannerChooser.php
index 849fbb5..5a74998 100644
--- a/includes/BannerChooser.php
+++ b/includes/BannerChooser.php
@@ -105,10 +105,24 @@
return;
}
 
-   // First pass allocate the minimum number of slots to each 
banner
+   // Sort the banners by weight, smallest to largest - this helps 
in slot allocation
+   // because we are not guaranteed to underallocate but we do 
want to attempt to give
+   // one slot per banner
+   usort( $this-banners, function( $a, $b ) {
+   return ( $a[ 'weight' ] = $b[ 'weight' ] ) ? 1 
: -1;
+   } );
+
+   // First pass allocate the minimum number of slots to each 
banner, giving at least one
+   // slot per banner up to RAND_MAX slots.
$sum = 0;
foreach ( $this-banners as $banner ) {
-   $slots = floor( ( $banner[ 'weight' ] / $total ) * 
self::RAND_MAX );
+   $slots = max( floor( ( $banner[ 'weight' ] / $total ) * 
self::RAND_MAX ), 1 );
+
+   // Compensate for potential overallocation
+   if ( $slots + $sum  self::RAND_MAX ) {
+   $slots = self::RAND_MAX - $sum;
+   }
+
$banner[ self::SLOTS_KEY ] = $slots;
$sum += $slots;
}

-- 
To view, visit https://gerrit.wikimedia.org/r/50931
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2214ed49ede39718bc22cb69e9662ad344beacc6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: Mwalker mwal...@wikimedia.org

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] Fix CentralNotice Underallocation - change (mediawiki...CentralNotice)

2013-02-26 Thread Adamw (Code Review) 
Adamw has submitted this change and it was merged.

Change subject: Fix CentralNotice Underallocation
..


Fix CentralNotice Underallocation

CentralNotice should have been giving at least 1 slot to each banner.
In the case where low weight banners were at the end of the banner
list they could be missed during the reallocation step.

This has been solved with sorting, explicit minimum, and taking
care of any potential overallocation.

Change-Id: I2214ed49ede39718bc22cb69e9662ad344beacc6
---
M includes/BannerChooser.php
1 file changed, 16 insertions(+), 2 deletions(-)

Approvals:
  Adamw: Verified; Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/BannerChooser.php b/includes/BannerChooser.php
index 849fbb5..5a74998 100644
--- a/includes/BannerChooser.php
+++ b/includes/BannerChooser.php
@@ -105,10 +105,24 @@
return;
}
 
-   // First pass allocate the minimum number of slots to each 
banner
+   // Sort the banners by weight, smallest to largest - this helps 
in slot allocation
+   // because we are not guaranteed to underallocate but we do 
want to attempt to give
+   // one slot per banner
+   usort( $this-banners, function( $a, $b ) {
+   return ( $a[ 'weight' ] = $b[ 'weight' ] ) ? 1 
: -1;
+   } );
+
+   // First pass allocate the minimum number of slots to each 
banner, giving at least one
+   // slot per banner up to RAND_MAX slots.
$sum = 0;
foreach ( $this-banners as $banner ) {
-   $slots = floor( ( $banner[ 'weight' ] / $total ) * 
self::RAND_MAX );
+   $slots = max( floor( ( $banner[ 'weight' ] / $total ) * 
self::RAND_MAX ), 1 );
+
+   // Compensate for potential overallocation
+   if ( $slots + $sum  self::RAND_MAX ) {
+   $slots = self::RAND_MAX - $sum;
+   }
+
$banner[ self::SLOTS_KEY ] = $slots;
$sum += $slots;
}

-- 
To view, visit https://gerrit.wikimedia.org/r/50931
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I2214ed49ede39718bc22cb69e9662ad344beacc6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: Mwalker mwal...@wikimedia.org
Gerrit-Reviewer: Adamw awi...@wikimedia.org
Gerrit-Reviewer: jenkins-bot

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits