[RFC|Accepted] Coding Guideline Modifications

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC] Coding Guideline Modifications

Post by naderman » Sun Jul 04, 2010 2:22 pm

bantu wrote:
  • "public, private, protected" should be used for class properties instead of "var"
Done.
bantu wrote:
  • Methods that are called statically should have the "static" keyword
That's kind of common sense, not adding that.
bantu wrote:
  • Use "static public function foo()" instead of "public static function foo()"
Done.
bantu wrote:
  • Prefer class constants over define().
Done.

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC] Coding Guideline Modifications

Post by bantu » Sun Jul 04, 2010 8:13 pm

Great. Looks good.

User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC] Coding Guideline Modifications

Post by naderman » Sun Jul 04, 2010 11:00 pm

It's been merged into develop.

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC] Coding Guideline Modifications

Post by bantu » Wed Jul 07, 2010 2:16 pm

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

User avatar
ToonArmy
Registered User
Posts: 335
Joined: Fri Mar 26, 2004 7:31 pm
Location: Bristol, UK
Contact:

Re: [RFC] Coding Guideline Modifications

Post by ToonArmy » Wed Jul 07, 2010 11:08 pm

The '@version $Id: $' bit in file headers should go as it makes no sense in Git.
Chris SmithBlogXMOOhlohArea51WikiNo support via PM/IM
Image

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC] Coding Guideline Modifications

Post by bantu » Sun Jul 11, 2010 11:40 am

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?

User avatar
rxu
Registered User
Posts: 126
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

Re: [RFC] Coding Guideline Modifications

Post by rxu » Sun Jul 11, 2010 11:52 am

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

Code: Select all

$foo = new bar();
Image

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Coding Guideline Modifications

Post by igorw » Sat Sep 04, 2010 8:42 am

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.

User avatar
ToonArmy
Registered User
Posts: 335
Joined: Fri Mar 26, 2004 7:31 pm
Location: Bristol, UK
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by ToonArmy » Sat Sep 04, 2010 12:12 pm

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.
Chris SmithBlogXMOOhlohArea51WikiNo support via PM/IM
Image

User avatar
rxu
Registered User
Posts: 126
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

Re: [RFC|Accepted] Coding Guideline Modifications

Post by rxu » Mon Sep 06, 2010 8:49 am

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.
Image

Post Reply