r61181 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r61180‎ | r61181 | r61182 >
Date:21:02, 17 January 2010
Author:ialex
Status:resolved (Comments)
Tags:
Comment:
Per ^demon, follow-up r61173: remove useless "@return nothing"
Modified paths:
  • /trunk/phase3/includes/HistoryPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/HistoryPage.php
——@@ -25,7 +25,6 @@
2626 * Construct a new HistoryPage.
2727 *
2828 * @param $article Article
29 - * @return nothing
3029 */
3130 function __construct( $article ) {
3231 global $wgUser;
——@@ -59,8 +58,6 @@
6059
6160 /**
6261 * Print the history page for an article.
63 - *
64 - * @return nothing
6562 */
6663 function history() {
6764 global $wgOut, $wgRequest, $wgScript;

Follow-up revisions

RevisionCommit summaryAuthorDate
r61643Followup r61181, put @return nothing back on history(). Some people like it.demon19:46, 28 January 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r61173Fix some doxygen warningsialex16:34, 17 January 2010

Comments

#Comment by Simetrical (talk | contribs)   23:12, 17 January 2010

Why is this useless? It tells you the function doesn't have a return value, rather than having an undocumented return value.

#Comment by 😂 (talk | contribs)   12:35, 18 January 2010

Not having a @return indicates no return value to me. Although I suppose until everything's return value is documented, it's slightly ambiguous.

#Comment by Simetrical (talk | contribs)   16:23, 18 January 2010

I'd always be unsure whether it's just undocumented or actually nothing. Most of our functions aren't documented at all, it would be really optimistic to just assume that the function you're looking at is . . .

#Comment by Siebrand (talk | contribs)   23:31, 18 January 2010

I tend to agree with Simetrical. Instead of RTFS-ing, you can immediately see you do not have to expect anything back. Suggesting revert and be done with it. According to ohloh we are already underdocumented compared to similar projects. Let's not make it worse...

#Comment by Nikerabbit (talk | contribs)   11:28, 19 January 2010

Ohloh is exaggerating since it counts all the i18n files as code.

#Comment by Simetrical (talk | contribs)   15:07, 19 January 2010

We should get them to fix that . . . it really skews the numbers unreasonably. 

130 10:05:57 /var/www/git-trunk/phase3$ find languages/messages/ -type f -exec cat {} + | wc -l
524199
0 10:06:04 /var/www/git-trunk/phase3$ find -type f -exec cat {} + | wc -l
846374
0 10:06:36 /var/www/git-trunk/phase3$ find -type f -name '*.php' -exec cat {} + | wc -l
741707

60% of our code is in languages/messages/, 70% if you only count PHP files -- obviously with few to no comments.

#Comment by IAlex (talk | contribs)   17:03, 19 January 2010

And the statistics cover the whole trunk, including extensions, other softwares we have there, ...

Also I find pointless to have a "@return" on the constructor.

#Comment by Tim Starling (talk | contribs)   05:15, 28 January 2010

Feel free to mark this resolved or OK or whatever when you're done arguing.

#Comment by Simetrical (talk | contribs)   19:22, 28 January 2010

Could you just decide it instead so we don't have to bikeshed over it? That's what Brion always did in this sort of case, and it saved a lot of pointless bickering.

#Comment by 😂 (talk | contribs)   19:49, 28 January 2010

I put it back on history() in r61643. Not on the constructor.

Status & tagging log