PFSenseDevWiki: Developer Rules

Developer Rules

Never commit untested code.
You break the code, you fix it. Period. Even if your code breaks another subsystem. Your code caused the fallout. Yes this is not glamorous but it is the right thing to do (or back your code out until a proper fix can be obtained).
If you commit a kernel change that requires a userland configuration change of any type, then you also must make sure there is PHP code to control the userland change as needed (different sysctls, different means of configuring something, etc.).
Avoid changing the XML configuration structure whenever possible. Where that is infeasible, do not commit code that changes the XML configuration structure without adding configuration upgrade code at the same time.
Never commit anything to any branch that knowingly causes a regression.
Any major or high risk changes must first be done in a git clone and reviewed by another committer before merging into mainline.
Pre-commit authorization such as in OpenBSD isn't used in pfSense except in the case of major changes. We do however run sanity checks on committed files and use access control to keep commits to authorized branches.

Note: Wrong HTML code is broken code. To break the code isn't allowed. A C compiler for example would complain in most cases if you break the code syntactically. So just because most browser are ignoring wrong code does not mean that you are not in charge to fix the code you did break.

pfSense uses the XHTML doctype in its webGUI code. The doctype enforces you to code against the following ruleset:

please use lower case tag names and not a mix of upper case and lower case tag names
breaks need to be closed (<br />)
image tags need to be closed (<img />)
an image tag "always" has an alt attribute, tho it may contain no value
horizontal rule tags need to be closed (<hr />)
HTML form fields need to be closed (<input />)
ampersands in an URL (e.g. within a href attribute) need to be coded as a HTML entity (e.g. &)
special characters (e.g. umlauts) need to be coded as HTML entities. see: http://en.selfhtml.org/html/referenz/zeichen.htm (in German, sorry. but the table is self explanatory)
a <table /> tag does not have a name attribute "period"
a <div /> tag does not have a name attribute "period"
a <ul /> tag does not have a name attribute "period"
a <li /> tag does not have a name attribute "period"
checkbox checked attributes need to be coded as checked="checked"
HTML field disabled attributes need to be coded as disabled="disabled"
HTML field readonly attributes need to be coded as readonly="readonly"
any HTML <input /> field has a type attribute (e.g. type="text")
same applies for nowrap (nowrap="nowrap") etc.
opening <p>, <b> tags do have a matching closing tag (e.g. </p>)
<table /> tags do not contain a <form /> tag.
please place the inputerror <div /> after the "tabnavtbl" < td />, before the "mainarea" <dic />
please place the "iform" <form /> tag right before the "tabcont&quot <table /> tag
there's no reason to include the inputerror <div /> twice within the same HTML file
the "type" attribute of a <form /> tag needs to contain a lower case value (e.g. type="post" or type="get")
the language=JavaScript attribute for <script /> tags is deprecated. Use the type="text/javascript" attribute instead.
always use lowercase attribute names for calling JavaScript events (e.g. onclick="foobar();")
the <embed /> tag is deprecated use <object /> instead
if you assign style attribute to a HTML element like this : element.style.borderTop = "2px solid #990000"; it needs to be enclosed by quotes

It's possible to syntax check you code in Firefox with the HTML validator plugin. Or you can simple use the W3C validator. The latter is supported by Opera even for RFC 1918 networks. Validom is a validator, thatdoes not validate against SGML. The validome guys do pretend, that this produces better results.

PHP Specific Rules

Rule #1:

Every time a rule is not followed, there must be good reason for this.

General rules

All php files must start with an (english) header block
Use descriptive (english) variable names
Use lower-case variable names ($my_very_long_var_name) or Java-style names ($myVeryLongVarName)
Add (english) comments, whenever necessary or helpful
Add "TODO:" comments, when there's something to be done
Add "FIXME:" comments, when there's something broken
add "NOTE:" comments, when there's something other people should know
Try to code in a readable way:

$header = "<head>$foo</head>";
$message = "SOME{$bar}TEXT";

is easier to read than

$header="<head>".$foo."</head>";
$message = "SOME" . $bar . "TEXT";

Try to simpify your code for better readability:

if ($bool1)

if ($bool2)

if ($bool3)

do_it();

whatever();

sould be written as:

if ($bool1 && $bool2 && $bool3)

do_it();

whatever();

Do not set unnessesary variables:

$is_set = isset($var);
if ($is_set) ...

Loop variables are $i, $j, $k, ...
All switch statements should have a default
Do not make explicit comparisons to boolean values:

while ($condition = = true) ...

In classes, use "private", "protected" and "public", not "var" for attribute declaration
Do not create source code lines longer than 80 chars
Do not to use deprecated or obsolete functions
If a PHP-internal function is an alias for another function, use the original (i.e. use exit() instead of die())

Indent style

Use "K&R style" (see http://en.wikipedia.org/wiki/Indent_style)

if ($x = = $y) {

something();
...

} else {

somethingelse();
...

}
finalthing();

There shouldn't be a space between a function name and its argument list:

isset($myvar);

... but separate function arguments with a single space:

do_something($foo, 27, false);

Use tabs for intendation (not spaces or a mixture of both)
... but use spaces in the middle of a line

config.xml

Boolean values which are false should be un-set:

$config['system']['enablesshd'] = "no";

should be:

unset($config['system']['enablesshd']);

PFSenseDevWiki : DeveloperRules

Developer Rules

HTML Specific Rules

PHP Specific Rules