Policies/Suggested Review Criteria

Code review should be constructive and specific. These criteria are a guideline for what reviewers should consider when reviewing new code. This is not a strict prerequisite to be completed before review. They are just for information purposes to be aware of what aspects of new code have actually been reviewed, and what has not. It is a list of suggestions for reviewers to record the criteria they're reviewing, rather than another barrier for developers of new applications and libraries to worry about. For example a reviewer does not have to test the new code on all platforms or compilers, but can say which ones were tested if more than the most common linux/gcc. See origins here.

All criteria do not all have to be completed, and additional criteria can optionally be defined by the reviewer. The desired result of a review is that the reviewer states whether the code is of enough quality as it is now, or whether changes are necessary, and what criteria were considered in making that assessment.

Library And Application Code

General

• The application / library works and performs the expected function.
• The library / application uses existing kdelibs classes where appropriate instead of reinventing the wheel.
• The application includes all necessary cmake checks for locating dependencies.
• Qt classes are not used where a KDE class is more appropriate (KFileDialog vs QDFileDialog, QHttp vs KIO etc).

Maintainability

• The application / library is designed for maintainability and is consistent with KDE standards and norms.
• Complex algorithms and optimizations are sufficiently commented to be understandable.
• Methods are public, protected, virtual and const as appropriate.
• Dependencies are still in active development

Security

• The application / library has no obvious security flaws.
• html entities ('<', '>', '"', '&') accepted from external sources are encoded.
• All places where the application launches external programs should be checked.
• Any SQL queries processing untrusted data should be checked. Queries need to be escaped appropriately to avoid sql injection.

Quality

• The application / library passes all krazy tests. All exceptions in a .krazy file are justified.
• Exception handling is done right.
• Classes are threadsafe where appropriate.
• Appropriate data structures are used ( eg, QMap or QHash etc)

Portability

• The new code has no unnecessary platform specific code, such as reading /proc etc. This does not apply to new code which is relevant only on specific platforms.
• The new code uses syntax which compiles on all supported compilers, free from gcc-isms etc.
• The application compiles and runs on Windows, Mac, BSD, Solaris.

Documentation and comments

• Comments in the code are appropriate and not excessive.
• Application user documentation is written.

Style

• Coding style in consistent with the rest of the library / module. Some modules (kdepim, kdelibs) have style rules to conform to, but most do not.
• Variable names are consistent with the target module or library module. (m or m_ prefix, camel casing, descriptive variable names instead of a, b and c2 etc)
• There are no excessively large methods. Methods longer than 40 lines can be broken into multiple methods if possible.
• There are no excessively long lines. for lines greater than 80 characters it might be possible to use intermediate variables more.

i18n and i10n

• One word strings use context for translation
• Strings are free from word puzzles (see Development/Tutorials/Localization/i18n_Mistakes)
• The KUIT system is used Development/Tutorials/Localization/i18n_Semantics.

Libraries Only

Maintainability

• All public classes use a private d-pointer class.

Documentation

• All public methods are documented.
• Private methods and classes are documented where necessary and marked as @internal.