[RFC|Accepted] Coding Guideline Modifications

[Oleg]

Another issue: are new functions required to not use globals and instead have them dependency injected?

[igorw]

I don't think we should be so strict about commit messages. It really slows down development. Having meaningful messages is important, but things like capitalization, spelling and too long lines really shouldn't require rebases every time.

[nickvergessen]

Another issue: are new functions required to not use globals and instead have them dependency injected?

I don't really like this one. We have globals everywhere in the code base. And I prefer it much more than having 6+ additional parameters on a function.

[imkingdavid]

Another issue: are new functions required to not use globals and instead have them dependency injected?

I don't really like this one. We have globals everywhere in the code base. And I prefer it much more than having 6+ additional parameters on a function.

+1

[bantu]

Another issue: are new functions required to not use globals and instead have them dependency injected?

I don't really like this one. We have globals everywhere in the code base. And I prefer it much more than having 6+ additional parameters on a function.

Your statement is missing a reason.

[Oleg]

I don't think we should be so strict about commit messages. It really slows down development. Having meaningful messages is important, but things like capitalization, spelling and too long lines really shouldn't require rebases every time.

If people write correct commit messages most of the time and occasionally make a mistake, it's no big deal to fix them. In projects that do not have commit message requirements I think you end up with a bit of a free for all as people spend that much less effort on commit messages.

You don't actually have to even go that far for proof, take a look at our own commit messages pre-git.

Where I agree this is a big issue is in contributors' commit messages. And the real problem I would say is that the developers can't fix contributors' commits in github pull requests.

[naderman]

There is a difference between having formal requirements for commit messages, which we have and enforcing a particular capitalisation in commit messages. Let's just move on.

[bantu]

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.

This has been changed now. See viewtopic.php?f=99&t=42353

[sajaki]

hi,

Is there a phpbb specific guideline on using docblocks or does this one apply ?
http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_phpDocumentor.howto.pkg.html#basics.docblock

[naderman]

Those are the rules for formatting them, yes.

We have a few more phpBB specific rules: Always document function return value using @return null if no value is returned. Always document all parameters with type and description. But I think that's all. Oh and we don't always document member variables, but often that is rather helpful (no need for @access though since we use PHP5 and require use of access modifiers anyway).