[RFC|Accepted] Coding Guideline Modifications

[naderman]

• "public, private, protected" should be used for class properties instead of "var"

Done.

• Methods that are called statically should have the "static" keyword

That's kind of common sense, not adding that.

• Use "static public function foo()" instead of "public static function foo()"

Done.

• Prefer class constants over define().

Done.

[bantu]

Great. Looks good.

[naderman]

It's been merged into develop.

[bantu]

The part about the version control system has to go into develop-olympus as well. This should be done rather sooner than later IMO.

[ToonArmy]

The '@version $Id: $' bit in file headers should go as it makes no sense in Git.

[bantu]

Do we want to regulate the use of the new operator?

Should it be

Code: Select all

$foo = new bar();

or

Code: Select all

$foo = new bar;

or should both be allowed?

[rxu]

Allowing both variants would be inconsistent imho. I'd suggest to leave the first one:

Code: Select all

$foo = new bar();

[igorw]

The file header still needs to be adjusted. Currently it is:

Code: Select all

/**
*
* @package {PACKAGENAME}
* @version $Id: $
* @copyright (c) 2007 phpBB Group
* @license http://opensource.org/licenses/gpl-license.php GNU Public License
*
*/

I'd suggest something among the lines of:

Code: Select all

/**
*
* @package {PACKAGENAME}
* @copyright (c) 2010 phpBB Group
* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License, version 2
*
*/

This should only apply to files new to the develop (ascraeus) branch.

[ToonArmy]

Could we use the build script to set the @version information to the release version, it's always been kind of handy as a quick check.

[rxu]

Under the Don't use uninitialized variables part of CG we instruct users about how to avoid warnings caused by uninitialized variables, but we don't really say how we intend to initialize them.
I'd suggest to adjust this part in the way like this:

Code: Select all

<h4>Don't use uninitialized variables.</h4>
For phpBB3, we intend to use a higher level of run-time error reporting. This will mean that the use of an uninitialized variable will be reported as a warning. These warnings can be avoided by two ways.
Initialize variables before use where it is possible, examples:

   <p class="bad">// Wrong </p>
   <div class="codebox"><pre>
foreach ($array as $key => $value)
{
   $foo[] = $key;
   $bar = $value;
}
   </pre></div>

   <p class="good">// Right </p>
   <div class="codebox"><pre>
$foo = array();
$bar = '';

foreach ($array as $key => $value)
{
   $foo[] = $key;
   $bar = $value;
}
   </pre></div>

Use the built-in isset() function to check whether a variable has been set - but preferably the variable is always existing. For checking if an array has a key set this can come in handy though, examples:

...and keep the further text as it follows.

Also could be applied to 3.0.x CG.