Development Guidelines

From Cloudrexx Development Wiki
(Redirected from Commit Policies)
Jump to: navigation, search

Introduction

Our Guidelines are based on the PEAR ones (PEAR 1 and PEAR 2 Coding standards). Please note, that the PEAR 2 standards are based on the PEAR 1 ones.

Since there are some things missing and some things not suitable for our purpose, here are some more guidelines, extending the ones from PEAR 2. These are ment for all languages we use:

Why to use guidelines

From Wikipedia:

Reducing the cost of software maintenance is the most often cited reason for following coding conventions. In their introduction to code conventions for the Java Programming Language, Sun Microsystems provides the following rationale:

Code conventions are important to programmers for a number of reasons:

  • 40%-80% of the lifetime cost of a piece of software goes to maintenance.
  • Hardly any software is maintained for its whole life by the original author.
  • Code conventions improve the readability of the software, allowing engineers to understand new code more quickly and thoroughly.
  • If you ship your source code as a product, you need to make sure it is as well packaged and clean as any other product you create.

Documentation

For usage, code samples, pseudo-code and schema definitions we use the same scheme as unix does. Additionally, to avoid mixups, we wrap placeholders in greater than and less than symbols and write them in bold (<argument>).

Coding Guidelines

Be sure to follow at least the basic guidelines. For example the following (mostly from PEAR):

  • The charset to use is UTF-8 (this is important for some functions too, for example PHP's htmlentities($foo, ENT_QUOTES, CONTREXX_CHARSET))
  • Indenting is done with 4 spaces per indent, no tabs
  • We write our code in american English
  • Think before write
  • Make sure the line containing the closing bracket has the same indention as the line containing the matching opening bracket. If opening and closing bracket are not on the same line the opening bracket should be the last character of its line.

Naming

Use of meaningful names makes files more easy to find and code way easier to read for you and all others working on the same project! In order to achieve proper naming, we wrote some guidelines for this: Naming conventions

Use (class) constants

Where available use existing constants. Where useful, write your own class constants. Avoid declaring new constants in the global scope (no matter what language), to avoid future compatibility problems.

Fields like "mode" or "type" are good examples for using class constants!

Make use of compiler messages

Compilers normally are a powerful tool. Most compilers can be used to deliver detailed errors, warnings and notices. That makes it an easy task to fix all messages (including notices). By fixing all these messages, we avoid future problems (a good example are deprecation messages).

Be sure to declare your variables properly!

Proper escaping

Proper escaping (for example SQL database, table and field names) avoid problems with reserved words. Since bugs caused by reserved words are often very hard to find, you should absolutely do proper escaping.

Avoid redundant code

Absurd or redundant (identical lines of code for more than once) make code harder to read and maintain (in a worst case scenario you will have to change the same line a hundred times).

Always avoid such misconseptions! Here are a few samples:

if (isset($foo) && !empty($foo)) {
// isset() is nonsense here, since empty does check the same itself

if (!empty($foo)) {
// means the same, but is easier to read
doSomething(1);
doSomething(2);
doSomething(3);
// This could easily be replaced by:

for ($i = 1; $i <= 3; $i++) {
    doSomething($i);
}
// which makes it easy to add another call and is way more easy to read for 10 and more calls
doSomething(1);
doSomething(4);
doSomething(19);
// Again could easily be replaced by:

foreach (array(1, 4, 19) as $number) {
    doSomething($number);
}
// which makes it easy to add another call and is way more easy to read for 10 and more calls

Be aware of compatibility

Before releasing a software, be sure to support all systems that will/could be used. For Web-Software, take special care about different browsers and -versions. Make sure new versions of software you rely on are supported too (for example PHP, MySQL, Apache, Windows/Linux, etc.)

Comments

  • Explain why things are done and not how things are done.
  • If you follow the guidelines the code itself explains what is done. However, for complicated code samples (which can not be made easier) you should also comment what the code is doing.
  • To explain why things are done, use examples or use cases.
  • Comments should not be used to backup old code (therefore we have a VCS). If necessary, you can add a short comment how it worked before but delete the old code.
  • Always comment before and while coding and not after coding.

Use visibility correct

The open/closed principle states "software entities should be open for extension but closed for modification". This means that an entity can allow its behavior to be extended without modifying its source code.

  • Always use the smallest visibility that still guarantees this principle.
  • Therefore in PHP you should use protected over private in general.
  • Anyway, there are exceptions, in which private can be useful, but you must only declare an attribute private if you have a getter and a setter for it.

Also don't use globals, since they can interfere with other parts of the software.are.

Write efficient, but readable code

Your code should run efficient and must not consume unnecessary CPU power. However, your code must still be readable for a normal human being.

Order in Conditional Statements

Do not use Yoda conditions when checking a variable against an expression.

Setter and Getter

A setter does not have any return value while a getter must always have a return value. Furthermore, a getter should never change the object in any way.

To ensure the correct value of any attribute you should also use the setter and getters in the class itself instead of using the attribute directly.

Single responsibility

A class should have only a single responsibility and therefore only one reason to change.

Do not mix different concerns into the same class (e.g calculating the yearly benefits and outputting them should be in separate classes).

Methods should also have only one responsibility and therefore only one reason to change.

Workflow Guidelines

For each bug you're going to fix or feature you'll implement, you should adhere to the following practice:

  1. Search if the issue already exists in the issue tracker. A search based on component is advised.
  2. If not yet present, do create an issue in the issue tracker.
    • Localization: Use English for summary and description of the issue.
    • Security level: set appropriate (make public if the issue does not contain sensitive data or poses a security risk).
  3. Assign the issue to yourself.
  4. For bug reports: Ensure you can fully reproduce the reported issue.
  5. Fix the bug or implement the feature.
  6. Enforce quality checks.
  7. Create pull-request.
  8. Wait for approval of pull-request.
  9. Merge pull-request.
  10. Perform roll-out.
  11. Update any related documentation.
  12. Close the issue in the issue tracker

Write a change log

Writing a log about what has changed makes it easy to debug errors in the future since we can reconstruct when a bug could have arisen. In addition it's easy to show customers what's new according to a change log.

Commit Policies

  • The main branch is called main.
  • Each feature has its own branch, prefixed by the ticket number.
  • Commits should not happen to the main branch directly but only through pull-requests.
  • Make your commits as atomic as possible.
  • Always double-check (review changeset) what exactly you are going to commit before doing so.
  • The commit message must follow the conventional GitHub guidelines. Here's a short description of the most important rules and some examples:

Schema for the first line of any commit message: <value>(<module>): <message> (<reference>)

value can be one of the following [1]:

Value Use-case Description
build Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
ci Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
chore Used for trivial tasks such as initial commits or language variables copied from live. Other examples are: implementation (of an existing feature, which doesn't involve a fix) and configuration (like the .gitignore or .gitattributes) [2]
docs Documentation only changes
feat A new feature
fix A bug fix
perf A code change that improves performance
refactor A code change that neither fixes a bug nor adds a feature
revert If the commit reverts a previous commit, it should begin with revert:, followed by the header of the reverted commit. In the body it should say: This reverts commit <hash>., where the hash is the SHA of the commit being reverted. [3]
style Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
test Adding missing tests or correcting existing tests

module is the related component's name in lowercase.

message starts with a verb in imperative form in lowercase.

reference is the related ticket ID

The first line should not be longer than 72 characters. Separate subject from the body with a blank line and wrap text at 72 chars.

Examples

  • refactor(lang): remove obsolete dirs (EXAM-1)
  • fix(news): fix new redirect problem (#584)
  • fix(memberdir): add url-modifier (EXAM-2)

See also

References

Object Oriented Development

Exceptions

Please study the article When and How to Use Exceptions.

  • Use exceptions when an exceptional case happens (e.g. something that's not in the intent of the surrounding method). Do not use mixed data-types to return error states.
  • Try to throw specific exceptions (not just \Exception!).
  • If you define your own exception class try to find a suitable parent class for it (there are multiple exception classes defined by PHP itself or by Cloudrexx).
  • Catch specific exceptions so a reader of your code knows your intentions. Try not to catch \Exception directly. Only catch \Throwable if no other solution is possible.

In order for the class loader to find exception classes, the exception classes must meet one of the following requirements:

  • Exception is defined in its own file as any other class would be
  • Exception is defined in the class file it relates to. Example: \Cx\Core\ContentManager\Model\Entity\PageException is defined in the file of class \Cx\Core\ContentManager\Model\Entity\Page (<base_class_fqcn>Exception).

Use statements

Use statements are to be avoided in general in favor of code readability and maintainability (code can be re-factored by using simple file operations). However, to access classes within the same component namespace (\Cx\<component_type>\<component_name>\*) use statements are not desired nor forbidden.

Static vs Self

In general, do use static:: (instead of self::) to ensure full class inheritance functionality.

Language Specific Guidelines

PHP

  • Use
    declare(strict_types=1);
    
    in all new files when writing code for PHP 7.2 or newer. For older versions it must not be added.
  • As of PHP 7.4 use strict typing for all new code (properties, arguments and return types). Since all variables are therefore strictly typed, type prefixes are to be omitted!
  • Use single quotes (instead of double quotes). Avoid functional strings, heredoc and nowdoc.
  • Do not add a PHP end tag (?>).
  • Document code according to PHP Code & API documentation. Everything always needs a DocBlock as we want to be able to generate a meaningful documentation from it.
    • Third-party code may omit documenting obvious variables, methods and constants.
  • Variable-length argument lists are prohibited.
    • Third-party code may use it.

SQL

  • Never select all columns of a table using the asterisk wildcard for the following reasons. Instead list explicit every column that you need to select, even if you need all columns of a table to be returned.
    • Using an asterisk slows down the database needlessly, because it first has to look up what columns are present.
    • To simplify the debugging of backwards-incompatible changes.
  • Use INNER JOINS where applicable. Reason:
MySQL does "only" know INNER or OUTER joins. In the context of MySQL, when someone talks about simple joins, he usually refers to INNER joins.
The keyword JOIN in MySQL is equivalent as to use LEFT OUTER JOIN, which is less efficient.
An INNER JOIN builds a 1-1 relation of data-sets. Whereas an OUTER JOIN builds a 1-n (for left joins and n-1 for right joins) relation of data-sets. For this reason, a INNER JOIN is always more efficient if you are looking exclusively for 1-1 relation of data-sets.
So in MySQL the expression JOIN is an equivalent to LEFT OUTER JOIN and not, as one would assume, a simple, INNER JOIN.

Product Specific Guidelines

Cloudrexx

Cloudrexx code is structured in components. For component specific guidelines see Component Definition.

Prerequisites

All code must comply with the current System Requirements.

Internationalization and encoding

In order to write software to be used in more than one country/timezone and/or supporting multiple languages use UTF-8 (also in databases, etc.) and store times and dates depending on the timezone.

Never write output strings in the code directly but use:

  • Text-Variables for all text displayed to end-users.
  • Class constants for text intended for developers or for automated parsing.

File/Resource Management

File Operations

Always use the FileSystem library for performing file operations like add/edit/delete on files and folders. This will ensure that neither you nor the end-user will have to deal with file permission issues.

File Upload

Use the Uploader to implement a file upload functionality.

File/Folder Listing

Use the MediaBrowser to list the content of a folder.

File Inclusion

Two rules:

  1. You must not use include(), include_once(), require() or require_once() to include a PHP class . All PHP classes are automatically being loaded by the ClassLoader.
  2. When dealing with files or resources you should always use ClassLoader::getFilePath() (or ClassLoader::getWebFilePath()) for accessing them. This will ensure that the concept of Customizing will work properly.
Examples
// load PHP class
// STOP: you must not load a PHP class manually! Read rule #1 again
// load data of a source file
$file = 'core_modules/Stats/Data/mobile-useragents.inc';
$file = \Cx\Core\Core\Controller\Cx::instanciate()->getClassLoader()->getFilePath($file);
$data = file_get_contents($file);
// output path to a web resource (image)
$imagePath = 'core_modules/Media/View/Media/Pdf.png';
$imagePath = \Cx\Core\Core\Controller\Cx::instanciate()->getClassLoader()->getWebFilePath($imagePath);
echo '<img src="' . $imagePath . '" />';

Input / Output Handling

Properly sanitize data read from the user as well as data sent to the user. See article Input / Output Handling.

GUI / UX

  • Add tooltips to functions/options that are not self-explaining.
  • HTML output of the Backend must be XHTML1.0 Transitional validated.

Model

Cloudrexx has two model abstractions:

The model of both abstractions must be defined in the file installer/data/contrexx_dump_structure.sql in SQL notation (DDL).

Important: The SQL DDL statements must adhere to the following rules:
  • No specific character set must named
  • No auto-increment value must be set

Doctrine

This is the default model abstraction and should be used whenever possible.

  • We use the "Deferred Explicit" change tracking policy. [1]
  • All entities that are Gedmo loggable must be marked as such by implementing the \Gedmo\Loggable\Loggable interface.
  • When defining relations in the YAML schema we always use "joinColumns" instead of "joinColumn" in order to avoid mixups and changes upon extension of the relation.

Legacy

The deprecated legacy model abstraction is based on the ADOdb database abstraction layer. Please use the Doctrine model abstraction whenever possible.

  • You must use the constant DBPREFIX as a table prefix in all of your queries:
    $this->cx->getDb()->getAdoDb()->Execute('SELECT 1 FROM `' . DBPREFIX . 'contrexx_core_mail_template`');
    

Events & Listeners

Events and Event Listeners should be registered in the related component hooks registerEvents and registerEventListeners.