r77762 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r77761‎ | r77762 | r77763 >
Date:04:22, 5 December 2010
Author:dantman
Status:ok (Comments)
Tags:
Comment:
Add method documentation for getCommonStylePath and getSkinStylePath.
Modified paths:
  • /trunk/phase3/includes/Skin.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Skin.php
@@ -2036,14 +2036,28 @@
20372037 }
20382038 }
20392039
 2040+ /**
 2041+ * Return a fully resolved style path url to images or styles stored in the common folder.
 2042+ * This method returns a url resolved using the configured skin style path
 2043+ * and includes the style version inside of the url.
 2044+ * @param $name String: The name or path of the common file to return the full path for.
 2045+ * @return String The fully resolved style path url including styleversion
 2046+ */
20402047 function getCommonStylePath( $name ) {
20412048 global $wgStylePath, $wgStyleVersion;
2042 - return "{$wgStylePath}/common/$name?$wgStyleVersion";
 2049+ return "{$wgStylePath}/common/$name?{$wgStyleVersion}";
20432050 }
20442051
 2052+ /**
 2053+ * Return a fully resolved style path url to images or styles stored in the curent skins's folder.
 2054+ * This method returns a url resolved using the configured skin style path
 2055+ * and includes the style version inside of the url.
 2056+ * @param $name String: The name or path of the skin resource file to return the full path for.
 2057+ * @return String The fully resolved style path url including styleversion
 2058+ */
20452059 function getSkinStylePath( $name ) {
20462060 global $wgStylePath, $wgStyleVersion;
2047 - return "{$wgStylePath}/{$this->stylename}/$name?$wgStyleVersion";
 2061+ return "{$wgStylePath}/{$this->stylename}/$name?{$wgStyleVersion}";
20482062 }
20492063
20502064 /* these are used extensively in SkinTemplate, but also some other places */

Follow-up revisions

RevisionCommit summaryAuthorDate
r78504Follow-up r77762 per CR, and an unrelated one-character whitespace fix which ...happy-melon18:30, 16 December 2010

Comments

#Comment by Nikerabbit (talk | contribs)   09:39, 5 December 2010

What's the logic of using {$foo} and $foo here? Only $this->stylename needs it and the others are inconsistent with $name.

+	 * @param $name String: The name or path of the skin resource file to return the full path for.

You are stating three times what the method returns. "The name or path of a skin resource file" should be enough.

#Comment by Happy-melon (talk | contribs)   18:31, 16 December 2010

Standardised the other way in r78504.

Status & tagging log