User:Catrope/Extension review/ArticleFeedbackv5

Base revision:

ArticleFeedbackv5.php

 * Line 82, 103: please be aware that tracking bucketing generates a lot of noise in the tracking data, to the point where it brought the site down once. This can only be enabled on the cluster if we use UDP logging (which we want to do anyway, I just need to get around to enabling it), and I will probably want to have it turned off initially when deploying.
 * Line 119-129: the keys in the bucket configuration (-, A, B, C) don't match the keys in the documentation comment (0, 1, 2, 3)
 * Line 161: are you guys even using the survey stuff anymore? If not, it should be stripped out of v5

ArticleFeedbackv5.hooks.php

 * Line 293-297: debugging code, please comment out or remove. This seems to be unfinished edit tagging functionality?

ArticleFeedbackv5.i18n.php

 * Line 35, 50: inconsistency between  (with spaces) and   (without spaces)
 * Line 39: message key is misspelled (invalud). The correct spelling is used in api/ApiViewFeedbackArticleFeedbackv5.php, which means that code will hit a missing message
 * Line 42: don't capitalize 'Feedback'. Our i18n people don't like Title Case, and 'feedback' isn't capitalized in any of the other messages
 * Our i18n people also prefer 'page' over 'article' but I think Fabrice raised a similar point already
 * Line 85: Don't Use Title Case In 'Very Good'

SpecialArticleFeedback5.php
Overall it looks like this special page is very much unfinished, and it is undeployable in its current state. This either needs serious work before deployment, or needs to be disabled by commenting it out from $wgSpecialPages.
 * Line 17, 48-50, 53, 55, 56, 61: hardcoded English text
 * Line 22: The  thing will not work on Wikipedias, because on those wikis "Wikipedia:" is a namespace prefix rather than an interwiki prefix. There are also general issues with building wikitext this way. You should build links the proper way:
 * Here  is a Title object (see also my Title::newFromText example below) and the second parameter is HTML (which is why you have to escape the i18n message)
 * Line 43-44: " This is a terrible, terrible hack. I'm taking it out as soon as I stop being an idiot and sort this ResourceLoader thing out". Would this week be a good time to stop being an idiot? ;) If you have questions regarding ResourceLoader, I can answer them, I wrote half of it. I'll be on vacation Tue-Fri but should be reasonably responsive to e-mail.
 * Line 75: MediaWiki has built-in functionality for obtaining a page ID from a title, and unlike your pageIdFromTitle function it also works for namespaced titles like this page :)
 * Line 34, 35: I've told you before that you should not use MySQL's TIMESTAMP type. You also acknowledged that the modified field wasn't needed because the rows in this table aren't changed, but it's still there
 * With the exception of the first table, comments are few and far between. Please add a comment above every table to explain what it contains (this is missing even for the first table -- from the field names I can deduce that it contains individual ratings) and above every field name where the contents of the field aren't immediately obvious from their name (things like auto increment IDs don't need comments, but things like foreign keys, enums and most text fields do)
 * Line 34, 35: I've told you before that you should not use MySQL's TIMESTAMP type. You also acknowledged that the modified field wasn't needed because the rows in this table aren't changed, but it's still there
 * With the exception of the first table, comments are few and far between. Please add a comment above every table to explain what it contains (this is missing even for the first table -- from the field names I can deduce that it contains individual ratings) and above every field name where the contents of the field aren't immediately obvious from their name (things like auto increment IDs don't need comments, but things like foreign keys, enums and most text fields do)
 * With the exception of the first table, comments are few and far between. Please add a comment above every table to explain what it contains (this is missing even for the first table -- from the field names I can deduce that it contains individual ratings) and above every field name where the contents of the field aren't immediately obvious from their name (things like auto increment IDs don't need comments, but things like foreign keys, enums and most text fields do)

api/ApiArticleFeedbackv5Utils.php

 * Line 54:  is global'ed but   is used in the code (and defined in the setup file)

modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js

 * Line 1618:  is called but the setting is called