User "Awjrichards" changed the status of MediaWiki.r98296. Old Status: new New Status: ok
User "Awjrichards" also posted a comment on MediaWiki.r98296. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/98296#c23736 Commit summary: allow people to set expiration date for Special:HideBanners Comment: Looks good, but as a stylistic note, I'd recommend setting the default expiry time of 2 weeks when you create $wgNoticeHideBannersExpiration in CentralNotice.php. That way the default behavior is a lot clearer to someone configuring this stuff. Also, the comment above the definition of $wgNoticeHIdeBannersExpiration is a bit confusing - the way the processing code is written, you would actually need to configure $wgNoticeHideBannersExpiration to be the result of some kind of expression to do anything meaningful (or at least dynamic, like 'two weeks from now'). If you were to change your handling of determining the expiration time to using strtotime, you would greatly improve the userfriendly-ness/clarity of this feature. For instance, in SpecialHideBanners.php: <pre> $exp = strtotime( $wgNoticeHideBannersExpiration ); if ( !$exp ) { // throw an error, retry a default, etc } </pre> And in CentralNotice.php: // a string parsable by strtotime defining when the // banner cookie should expire $wgNoticeHideBannersExpiration = '+2 weeks'; Just a thought! _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
