From 163a0974a3b5f70f698c5eacbd5486891e238bc4 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Fri, 25 Jun 2010 01:43:58 +0200 Subject: [PATCH 1/3] [task/coding-guidelines] Coding guideline update: Class names, eval, VCS, EOF - Class names need to be prefixed with phpbb_ - eval should not be used in any form - there should be newlines at the end of file - the closing php tag should be ommited - array elements should always have a trailing comma - the phpBB VCS is now git - removed the coding guidelines changelog PHPBB3-9557 --- phpBB/docs/coding-guidelines.html | 178 +++++++++++------------------- 1 file changed, 67 insertions(+), 111 deletions(-) diff --git a/phpBB/docs/coding-guidelines.html b/phpBB/docs/coding-guidelines.html index 7f747e09e2..a1f52c757b 100644 --- a/phpBB/docs/coding-guidelines.html +++ b/phpBB/docs/coding-guidelines.html @@ -62,11 +62,12 @@
  • Code Layout/Guidelines
      -
    1. Variable/Function Naming
      2. +
    2. Variable/Function/Class Naming
    3. Code Layout
    4. SQL/SQL Layout
    5. Optimizations
    6. General Guidelines
      7. +
    7. Restrictions on the Use of PHP
  • Styling @@ -90,7 +91,7 @@
  • VCS Guidelines
    1. Repository structure
      2. -
    2. Commit messages
      3. +
    3. Commit Messages and Repository Rules
  • Guidelines Changelog
    • @@ -127,7 +128,7 @@

    Linefeeds:

    Ensure that your editor is saving files in the UNIX (LF) line ending format. This means that lines are terminated with a newline, not with Windows Line endings (CR/LF combo) as they are on Win32 or Classic Mac (CR) Line endings. Any decent editor should be able to do this, but it might not always be the default setting. Know your editor. If you want advice for an editor for your Operating System, just ask one of the developers. Some of them do their editing on Win32.

    -

    1.ii. File Header

    +

    1.ii. File Layout

    Standard header for new files:

    This template of the header must be included at the start of all phpBB files:

    @@ -145,6 +146,14 @@

    Please see the File Locations section for the correct package name.

    +

    PHP closing tags

    + +

    A file containg only PHP code should not end with the optional PHP closing tag ?> to avoid issues with whitespace following it.

    + +

    Newline at end of file

    + +

    All files should end in a newline so the last line does not appear as modified in diffs, when a line is appended to the file.

    +

    Files containing inline code:

    For those files you have to put an empty comment directly after the header to prevent the documentor assigning the header to the first code element found.

    @@ -290,7 +299,7 @@ PHPBB_QA (Set board to QA-Mode, which means the updater also c

    Please note that these Guidelines applies to all php, html, javascript and css files.

    -

    2.i. Variable/Function Naming

    +

    2.i. Variable/Function/Class Naming

    We will not be using any form of hungarian notation in our naming conventions. Many of us believe that hungarian naming is one of the primary code obfuscation techniques currently in use.

    @@ -322,6 +331,10 @@ for ($i = 0; $i < $outer_size; $i++)

    Function Arguments:

    Arguments are subject to the same guidelines as variable names. We don't want a bunch of functions like: do_stuff($a, $b, $c). In most cases, we'd like to be able to tell how to use a function by just looking at its declaration.

    +

    Class Names:

    + +

    Apart from following the rules for function names, all classes should be prefixed with phpbb_ to avoid name clashes.

    +

    Summary:

    The basic philosophy here is to not hurt code clarity for the sake of laziness. This has to be balanced by a little bit of common sense, though; print_login_status_for_a_given_user() goes too far, for example -- that function would be better named print_user_login_status(), or just print_login_status().

    @@ -471,6 +484,26 @@ $post_url = "{$phpbb_root_path}posting.$phpEx?mode=$mode&amp;start=$start";

    In SQL Statements mixing single and double quotes is partly allowed (following the guidelines listed here about SQL Formatting), else it should be tryed to only use one method - mostly single quotes.

    +

    Commas after every array element:

    +

    If an array is defined with each element on its own line, you still have to modify the previous line to add a comma when appending a new element. PHP allows for trailing (useless) commas in array definitions. These should always be used so each element including the comma can be appended with a single line

    + +

    // wrong

    + 
    +$foo = array(
+	'bar' => 42,
+	'boo' => 23
+);
+
    + +

    // right

    + 
    +$foo = array(
+	'bar' => 42,
+	'boo' => 23,
+);
+
    + +

    Associative array keys:

    In PHP, it's legal to use a literal string as a key to an associative array without quoting that string. We don't want to do this -- the string should always be quoted to avoid confusion. Note that this is only when we're using a literal, not when we're using a variable, examples:

    @@ -1043,6 +1076,22 @@ append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=group&amp;

    Your page should either call page_footer() in the end to trigger output through the template engine and terminate the script, or alternatively at least call the exit_handler(). That call is necessary because it provides a method for external applications embedding phpBB to be called at the end of the script.

    +

    2.vi. Restrictions on the Use of PHP

    + +

    Dynamic code execution:

    + +

    Never execute dynamic PHP code (generated or in a constant string) using any of the following PHP functions:

    + + + +

    If absolutely necessary a file should be created, and a mechanism for creating this file prior to running phpBB should be provided as a setup process.

    + +

    The e modifier in preg_replace can be replaced by preg_replace_callback and objects to encapsulate state that is needed in the callback code.

    +
    Back to Top
    @@ -2326,126 +2375,33 @@ if (utf8_case_fold_nfc($string1) == utf8_case_fold_nfc($string2))
    -

    The version control system for phpBB3 is subversion. The repository is available at http://code.phpbb.com/svn/phpbb.

    +

    The version control system for phpBB3 is git. The repository is available at http://github.com/phpbb/phpbb3.

    7.i. Repository Structure

    -

    7.ii. Commit Messages

    - -

    The commit message should contain a brief explanation of all changes made within the commit. Often identical to the changelog entry. A bug ticket can be referenced by specifying the ticket ID with a hash, e.g. #12345. A reference to another revision should simply be prefixed with r, e.g. r12345.

    - -

    Junior Developers need to have their patches approved by a development team member first. The commit message must end in a line with the following format:

    - - 
    -Authorised by: developer1[, developer2[, ...]]
-
    - -
    - -
    Back to Top
    - - - - -
    - -

    8. Guidelines Changelog

    -
    -
    - -
    -

    Revision 10007

    - - - -

    Revision 9817

    - -
      -
    • Added VCS section.
      • -
    - -

    Revision 8732

    - - - -

    Revision 8596+

    - -
      -
    • Removed sql_build_array('MULTI_INSERT'... statements.
      • -
    • Added sql_multi_insert() explanation.
      • -
    - -

    Revision 1.31

    - -
      -
    • Added add_form_key and check_form_key.
      • -
    - -

    Revision 1.24

    - - - -

    Revision 1.16

    - - - -

    Revision 1.11-1.15

    - -
      -
    • Various document formatting, spelling, punctuation, grammar bugs.
      • -
    - -

    Revision 1.9-1.10

    - - - -

    Revision 1.8

    - - - -

    Revision 1.5

    - - +

    7.ii. Commit Messages and Reposiory Rules

    +

    Information on repository rules, such as commit messages can be found at http://wiki.phpbb.com/display/DEV/Git

    .
    From e7cc707931518e98f06a804471ccce4f33f6773f Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 3 Jul 2010 15:41:09 +0200 Subject: [PATCH 2/3] [task/coding-guidelines] Added a section about class names. The class naming / autoloading RFC is located on area51: http://area51.phpbb.com/phpBB/viewtopic.php?f=84&t=33237 PHPBB3-9557 --- phpBB/docs/coding-guidelines.html | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/phpBB/docs/coding-guidelines.html b/phpBB/docs/coding-guidelines.html index a1f52c757b..e34e1d9929 100644 --- a/phpBB/docs/coding-guidelines.html +++ b/phpBB/docs/coding-guidelines.html @@ -333,7 +333,33 @@ for ($i = 0; $i < $outer_size; $i++)

    Class Names:

    -

    Apart from following the rules for function names, all classes should be prefixed with phpbb_ to avoid name clashes.

    +

    Apart from following the rules for function names, all classes should meet the following conditions:

    +
      +
    • Every class must be defined in a separate file.
      • +
    • The classes have to be located in a subdirectory of includes/.
      • +
    • Classnames to be prefixed with phpbb_ to avoid name clashes, the filename should not contain the prefix.
      • +
    • Class names have to reflect the location of the file they are defined in. The longest list of prefixes, separated by underscores, which is a valid path must be the directory in which the file is located. So the directory names must not contain any underscores, but the filename may. If the filename would be empty the last directory name is used for the filename as well.
      • +
    • Directories should typically be a singular noun (e.g. dir in the example below, not dirs.
      • +
    + +

    So given the following example directory structure you would result in the below listed lookups

    + 
    +includes/
+  class_name.php
+  dir/
+    class_name.php
+    dir.php
+      subdir/
+        class_name.php
+
    + + 
    +phpbb_class_name            - includes/class_name.php
+phpbb_dir_class_name        - includes/dir/class_name.php
+phpbb_dir                   - includes/dir/dir.php
+phpbb_dir_subdir_class_name - includes/dir/subdir/class_name.php
+
    +

    Summary:

    The basic philosophy here is to not hurt code clarity for the sake of laziness. This has to be balanced by a little bit of common sense, though; print_login_status_for_a_given_user() goes too far, for example -- that function would be better named print_user_login_status(), or just print_login_status().

    From d8ff43c080d4f9d097f74b51f329f86ba83499f6 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sun, 4 Jul 2010 16:16:13 +0200 Subject: [PATCH 3/3] [task/coding-guidelines] Class member qualifier guidelines Use private, protected or public instead of var. Use static public instead of public static. Use class constants instead of define(). PHPBB3-9557 --- phpBB/docs/coding-guidelines.html | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/phpBB/docs/coding-guidelines.html b/phpBB/docs/coding-guidelines.html index e34e1d9929..f340b671cc 100644 --- a/phpBB/docs/coding-guidelines.html +++ b/phpBB/docs/coding-guidelines.html @@ -696,6 +696,26 @@ switch ($mode) }
    +

    Class Members

    +

    Use the explicit visibility qualifiers public, private and protected for all properties instead of var. + +

    Place the static qualifier before the visibility qualifiers.

    + +

    //Wrong

    + 
    +var $x;
+private static function f()
+
    + +

    // Right

    + 
    +public $x;
+static private function f()
+
    + +

    Constants

    +

    Prefer class constants over global constants created with define().

    +

    2.iii. SQL/SQL Layout

    Common SQL Guidelines: