r76612 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r76611‎ | r76612 | r76613 >
Date:00:47, 13 November 2010
Author:reedy
Status:ok (Comments)
Tags:
Comment:
Fixup some more documentation
Modified paths:
  • /trunk/phase3/includes/Article.php (modified) (history)
  • /trunk/phase3/includes/ImageGallery.php (modified) (history)
  • /trunk/phase3/includes/Revision.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/includes/User.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/User.php
@@ -2188,7 +2188,7 @@
21892189 /**
21902190 * Check if user is allowed to access a feature / make an action
21912191 * @param $action \string action to be checked
2192 - * @return \bool True if action is allowed, else false
 2192+ * @return Boolean: True if action is allowed, else false
21932193 */
21942194 function isAllowed( $action = '' ) {
21952195 if ( $action === '' )
@@ -2206,7 +2206,7 @@
22072207
22082208 /**
22092209 * Check whether to enable recent changes patrol features for this user
2210 - * @return \bool True or false
 2210+ * @return Boolean: True or false
22112211 */
22122212 public function useRCPatrol() {
22132213 global $wgUseRCPatrol;
@@ -2711,7 +2711,7 @@
27122712
27132713 /**
27142714 * Get whether the user is blocked from using Special:Emailuser.
2715 - * @return \bool True if blocked
 2715+ * @return Boolean: True if blocked
27162716 */
27172717 function isBlockedFromEmailuser() {
27182718 $this->getBlockedStatus();
@@ -2720,7 +2720,7 @@
27212721
27222722 /**
27232723 * Get whether the user is allowed to create an account.
2724 - * @return \bool True if allowed
 2724+ * @return Boolean: True if allowed
27252725 */
27262726 function isAllowedToCreateAccount() {
27272727 return $this->isAllowed( 'createaccount' ) && !$this->isBlockedFromCreateAccount();
@@ -2729,7 +2729,7 @@
27302730 /**
27312731 * Get this user's personal page title.
27322732 *
2733 - * @return \type{Title} User's personal page title
 2733+ * @return Title: User's personal page title
27342734 */
27352735 function getUserPage() {
27362736 return Title::makeTitle( NS_USER, $this->getName() );
@@ -2738,7 +2738,7 @@
27392739 /**
27402740 * Get this user's talk page title.
27412741 *
2742 - * @return \type{Title} User's talk page title
 2742+ * @return Title: User's talk page title
27432743 */
27442744 function getTalkPage() {
27452745 $title = $this->getUserPage();
@@ -2747,7 +2747,7 @@
27482748
27492749 /**
27502750 * Get the maximum valid user ID.
2751 - * @return \int User ID
 2751+ * @return Integer: User ID
27522752 * @static
27532753 */
27542754 function getMaxID() {
@@ -2764,7 +2764,7 @@
27652765 /**
27662766 * Determine whether the user is a newbie. Newbies are either
27672767 * anonymous IPs, or the most recently created accounts.
2768 - * @return \bool True if the user is a newbie
 2768+ * @return Boolean: True if the user is a newbie
27692769 */
27702770 function isNewbie() {
27712771 return !$this->isAllowed( 'autoconfirmed' );
@@ -2772,8 +2772,8 @@
27732773
27742774 /**
27752775 * Check to see if the given clear-text password is one of the accepted passwords
2776 - * @param $password \string user password.
2777 - * @return \bool True if the given password is correct, otherwise False.
 2776+ * @param $password String: user password.
 2777+ * @return Boolean: True if the given password is correct, otherwise False.
27782778 */
27792779 function checkPassword( $password ) {
27802780 global $wgAuth;
@@ -2813,7 +2813,7 @@
28142814 /**
28152815 * Check if the given clear-text password matches the temporary password
28162816 * sent by e-mail for password reset operations.
2817 - * @return \bool True if matches, false otherwise
 2817+ * @return Boolean: True if matches, false otherwise
28182818 */
28192819 function checkTemporaryPassword( $plaintext ) {
28202820 global $wgNewPasswordExpiry;
@@ -2871,7 +2871,7 @@
28722872 *
28732873 * @param $val \string Input value to compare
28742874 * @param $salt \string Optional function-specific data for hashing
2875 - * @return \bool Whether the token matches
 2875+ * @return Boolean: Whether the token matches
28762876 */
28772877 function matchEditToken( $val, $salt = '' ) {
28782878 $sessionToken = $this->editToken( $salt );
@@ -2887,7 +2887,7 @@
28882888 *
28892889 * @param $val \string Input value to compare
28902890 * @param $salt \string Optional function-specific data for hashing
2891 - * @return \bool Whether the token matches
 2891+ * @return Boolean: Whether the token matches
28922892 */
28932893 function matchEditTokenNoSuffix( $val, $salt = '' ) {
28942894 $sessionToken = $this->editToken( $salt );
@@ -3048,7 +3048,7 @@
30493049 /**
30503050 * Is this user allowed to send e-mails within limits of current
30513051 * site configuration?
3052 - * @return \bool True if allowed
 3052+ * @return Boolean: True if allowed
30533053 */
30543054 function canSendEmail() {
30553055 global $wgEnableEmail, $wgEnableUserEmail;
@@ -3063,7 +3063,7 @@
30643064 /**
30653065 * Is this user allowed to receive e-mails within limits of current
30663066 * site configuration?
3067 - * @return \bool True if allowed
 3067+ * @return Boolean: True if allowed
30683068 */
30693069 function canReceiveEmail() {
30703070 return $this->isEmailConfirmed() && !$this->getOption( 'disablemail' );
@@ -3077,7 +3077,7 @@
30783078 * confirmed their address by returning a code or using a password
30793079 * sent to the address from the wiki.
30803080 *
3081 - * @return \bool True if confirmed
 3081+ * @return Boolean: True if confirmed
30823082 */
30833083 function isEmailConfirmed() {
30843084 global $wgEmailAuthentication;
@@ -3098,7 +3098,7 @@
30993099
31003100 /**
31013101 * Check whether there is an outstanding request for e-mail confirmation.
3102 - * @return \bool True if pending
 3102+ * @return Boolean: True if pending
31033103 */
31043104 function isEmailConfirmationPending() {
31053105 global $wgEmailAuthentication;
@@ -3527,7 +3527,7 @@
35283528 * @param $hash \string Password hash
35293529 * @param $password \string Plain-text password to compare
35303530 * @param $userId \string User ID for old-style password salt
3531 - * @return \bool
 3531+ * @return Boolean:
35323532 */
35333533 static function comparePasswords( $hash, $password, $userId = false ) {
35343534 $type = substr( $hash, 0, 3 );
Index: trunk/phase3/includes/Article.php
@@ -1758,7 +1758,7 @@
17591759 /**
17601760 * Update the page record to point to a newly saved revision.
17611761 *
1762 - * @param $dbw Database object
 1762+ * @param $dbw DatabaseBase: object
17631763 * @param $revision Revision: For ID number, and text used to set
17641764 length and redirect status fields
17651765 * @param $lastRevision Integer: if given, will not overwrite the page field
@@ -3609,12 +3609,12 @@
36103610 * Every 100th edit, prune the recent changes table.
36113611 *
36123612 * @private
3613 - * @param $text New text of the article
3614 - * @param $summary Edit summary
3615 - * @param $minoredit Minor edit
 3613+ * @param $text String: New text of the article
 3614+ * @param $summary String: Edit summary
 3615+ * @param $minoredit Boolean: Minor edit
36163616 * @param $timestamp_of_pagechange Timestamp associated with the page change
3617 - * @param $newid rev_id value of the new revision
3618 - * @param $changed Whether or not the content actually changed
 3617+ * @param $newid Integer: rev_id value of the new revision
 3618+ * @param $changed Boolean: Whether or not the content actually changed
36193619 */
36203620 public function editUpdates( $text, $summary, $minoredit, $timestamp_of_pagechange, $newid, $changed = true ) {
36213621 global $wgDeferredUpdateList, $wgMessageCache, $wgUser, $wgEnableParserCache;
Index: trunk/phase3/includes/parser/Parser.php
@@ -1613,7 +1613,7 @@
16141614
16151615 /**
16161616 * Process [[ ]] wikilinks
1617 - * @return processed text
 1617+ * @return String: processed text
16181618 *
16191619 * @private
16201620 */
Index: trunk/phase3/includes/Revision.php
@@ -798,7 +798,7 @@
799799 * Insert a new revision into the database, returning the new revision ID
800800 * number on success and dies horribly on failure.
801801 *
802 - * @param $dbw DatabaseBase (master connection)
 802+ * @param $dbw DatabaseBase: (master connection)
803803 * @return Integer
804804 */
805805 public function insertOn( $dbw ) {
Index: trunk/phase3/includes/Title.php
@@ -329,8 +329,8 @@
330330 * This will only return the very next target, useful for
331331 * the redirect table and other checks that don't need full recursion
332332 *
333 - * @param $text \type{\string} Text with possible redirect
334 - * @return \type{Title} The corresponding Title
 333+ * @param $text String: Text with possible redirect
 334+ * @return Title: The corresponding Title
335335 */
336336 public static function newFromRedirect( $text ) {
337337 return self::newFromRedirectInternal( $text );
@@ -595,21 +595,21 @@
596596 /**
597597 * Get the main part with underscores
598598 *
599 - * @return \type{\string} Main part of the title, with underscores
 599+ * @return Stromg: Main part of the title, with underscores
600600 */
601601 public function getDBkey() { return $this->mDbkeyform; }
602602
603603 /**
604604 * Get the namespace index, i.e.\ one of the NS_xxxx constants.
605605 *
606 - * @return \type{\int} Namespace index
 606+ * @return Integer: Namespace index
607607 */
608608 public function getNamespace() { return $this->mNamespace; }
609609
610610 /**
611611 * Get the namespace text
612612 *
613 - * @return \type{\string} Namespace text
 613+ * @return String: Namespace text
614614 */
615615 public function getNsText() {
616616 global $wgContLang;
@@ -4029,7 +4029,7 @@
40304030 * In other words, is this a content page, for the purposes of calculating
40314031 * statistics, etc?
40324032 *
4033 - * @return \type{\bool}
 4033+ * @return Boolean
40344034 */
40354035 public function isContentPage() {
40364036 return MWNamespace::isContent( $this->getNamespace() );
Index: trunk/phase3/includes/ImageGallery.php
@@ -76,7 +76,7 @@
7777 /**
7878 * Set the caption (as HTML)
7979 *
80 - * @param $caption Caption
 80+ * @param $caption String: Caption
8181 */
8282 public function setCaptionHtml( $caption ) {
8383 $this->mCaption = $caption;

Follow-up revisions

RevisionCommit summaryAuthorDate
r76624Followup r76612, nfi what a stromg isreedy12:50, 13 November 2010

Comments

#Comment by Nikerabbit (talk | contribs)   10:31, 13 November 2010

Uh? For which system are you fixing this? \type \bool etc are annotations for doxygen defined in doxygen.conf

+	 * @return Stromg: Main part of the title, with underscores
#Comment by Platonides (talk | contribs)   17:47, 15 November 2010

Probably for PHPDoc compatibility

#Comment by Nikerabbit (talk | contribs)   17:56, 15 November 2010

The linked part also says We should probably drop PHPDoc compatibility. But what do I now, Doxygen output is very dull and boring regardless.

#Comment by 😂 (talk | contribs)   17:57, 15 November 2010

FWIW, I would much rather see "@return String" than "@return \type{\string}"

The latter isn't as easy to read.

#Comment by Nikerabbit (talk | contribs)   18:02, 15 November 2010

http://svn.wikimedia.org/doc/classApiQueryBase.html#a6c2df65d597a3daeecf0aa42dba0a6c7 vs. http://translatewiki.net/docs/Translate/html/classHtmlTag.html#d57bd4cd7801a87bf3e9319bf77cc1cc

Preferably it should be readable in both in generated documentation and in the source code. That seems to be impossible to do with Doxygen.

#Comment by Nikerabbit (talk | contribs)   18:04, 15 November 2010

Well, \string should be enough. Please look at the defined aliases in the config file or how it is done in Translate extension.

#Comment by Reedy (talk | contribs)   18:00, 15 November 2010

And when we get \type{\Array(string)} or whatever it is, it gets worse

I've also logged Write style guide for function inline commenting

Status & tagging log