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.
HTML Specific Rules
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" <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:
- Try to simpify your code for better readability:
- Do not set unnessesary variables:
- Loop variables are $i, $j, $k, ...
- All switch statements should have a default
- Do not make explicit comparisons to boolean values:
- 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())
$header = "<head>$foo</head>";
$message = "SOME{$bar}TEXT";
is easier to read than$message = "SOME{$bar}TEXT";
$header="<head>".$foo."</head>";
$message = "SOME" . $bar . "TEXT";
$message = "SOME" . $bar . "TEXT";
if ($bool1)
sould be written as:
if ($bool2)
whatever();
if ($bool3)
do_it();
if ($bool1 && $bool2 && $bool3)
do_it();
whatever();
$is_set = isset($var);
if ($is_set) ...
if ($is_set) ...
while ($condition = = true) ...
Indent style
- Use "K&R style" (see http://en.wikipedia.org/wiki/Indent_style)
- There shouldn't be a space between a function name and its argument list:
- ... but separate function arguments with a single space:
- Use tabs for intendation (not spaces or a mixture of both)
- ... but use spaces in the middle of a line
if ($x = = $y) {
finalthing();
something();
...
} else {...
somethingelse();
...
}...
finalthing();
isset($myvar);
do_something($foo, 27, false);
config.xml
- Boolean values which are false should be un-set:
$config['system']['enablesshd'] = "no";
should be:
unset($config['system']['enablesshd']);