User:APatro (WMF)/Patch Review Feedback

Contains a list of comments I've received on the patches that I've submitted. I've also added a few general guidelines that will help avoid silly issues such as formatting and failing tastes when submitting patches.

Some of the instructions here are specific to working with the Translate extension.

General guidelines

 * Always ensure you run  before submitting patch.
 * Also run  to verify any non-auto-fixable issues with the code.
 * In case of UI changes test with atleast the Timeless and Vector skin.
 * If a patch fixes multiple bugs, each bug should be listed in a separate line -
 * When working on a big patch, keep an eye on the debug logs as sometimes warnings get logged there, and may not be shown on the user interface.
 * When adding scripts that perform an irreversible changes by default. Provide a  flag that allows users to run a dry-run explaining what the script would do, and allow manual inspection before actually performing the operation.

Programming guidelines

 * Use of  is discouraged. Use   or comparison to   to be more explicit.
 * Warnings should never be suppressed for a large section of code.
 * Throw an exception incase of unintended effects. Use specific exceptions from here - http://php.net/manual/en/spl.exceptions.php
 * Ignore parenthesis when using instanceof
 * Avoid  when it's not necessary.   should work well here. For eg, use    or   instead of
 * Avoid  for values that are known to be defined. Comparison to   is more explicit and avoids hard to notice errors caused by spelling mistakes.
 * If a class is in a namespace, and uses another class in a different namespace multiple prefer to add a  statement at the top of the file rather than using the fully qualified class name multiple times.
 * is safer when compared to .    returns true for private functions as well.

Unit Tests

 * Run the test cases for the Translate extension before submitting the patch -
 * Use PHPUnit method -  instead of checking
 * Use  when adding hooks for test cases.
 * When using, using   is more readable then returning a huge array.

PHP Doc / Comments

 * - The convention is to use all lowercase for primitive types and TitleCase for objects. For eg:  instead of
 * - Array of strings, including associative arrays can be denoted by - . Same for other types of arrays.
 * Instead of writing  remove the   statement entirely.
 * In the case the function declaration spans multiple lines, prefer to have the  is on it's own line.


 * TBD

Translations

 * You shouldn't update the other language files besides  and  . Rest of the files will update automatically once the changes go through translatewiki.net.

JavaScript

 * Run  in the   directory before submitting the patch.
 * When declaring function documentation - Please leave an empty line between the previous code / function and the comment.
 * Do not use the word extension or component. Instead use module in comments or commit messages.
 * Per Manual:Coding conventions/JavaScript only global variables should use  - object properties and local variables should compare to   directly.