OPNsense Forum

English Forums => Development and Code Review => Topic started by: 8191 on December 07, 2015, 08:02:50 am

Title: Coding guidelines
Post by: 8191 on December 07, 2015, 08:02:50 am
Are there any coding guidelines for OPNsense? The forked pfsense code does not seem to have any guidelines (e.g. indents vary in tabs and spaces, single if statements used with and without curly braces, etc.)... Currently the code is a mix between different coding styles, it seems.

If one adds code to existing files, should the style of the file being adopted, or is there a OPNsense recommendation to apply? Shall existing files be adopted to a consistent style in future?
Title: Re: Coding guidelines
Post by: AdSchellevis on December 07, 2015, 11:26:46 am
The short answer is, PSR1/2 for PHP code and following the peps for python based code.
We have some documentation available on our wiki about our architecture and used components, but it's probably better if I explain a bit more about that.

Maybe I should start by explaining the system we want eventually and the path we see in getting there.
Our ideal OPNsense system would look like a standard FreeBSD system using our pluggable user interface for management, which supports both real users as "machine" users (REST).
When developing we want the code to be clean and coded as DRY as possible and don't want to invent the wheel when not needed.
And we do want the user interface to be able to run as non-root user instead of root by restructuring the way commands are passed to the system (configd).

But, then there is reality, in the sense of a system left around without code maintenance for a very long time and we need to transition that into something more structured.

One of the first things (on the programming part of the system) we did was build components around an existing framework (Phalcon) to create new modules, which could use validated configuration data (from the config.xml), supply a restfull api and generate html output using standard templates (Volt).
We created the configd system, which could generate system configuration and execute system calls using predefined templates.
And then we started using those new components for our first newly designed modules (like the proxy and the traffic shaper).
More information about the “to-be” architecture can be found on our wiki page (https://wiki.opnsense.org/index.php/Develop:Architecture (https://wiki.opnsense.org/index.php/Develop:Architecture))

Knowing we can’t change the world in a single day and having a lot of legacy to drag around with us, our strategy consists of three parts:

1)    Cleanup and maintenance
Restructure old (legacy) code, basically all code in the src/www, src/etc/inc to make it better readable, easier to use and remove unused / unnecessary parts. By doing so we want to extend the lifetime of the old code a bit and make the transition in new code easier eventually.

2) Detach
Move system configuration calls to configd where possible, which gives the administrator the advantage of running those commands from the command line and helps removing the need for root user access in the future. The ipsec VICI implementation is one example of this stage.

3) Moving on   
(re)build new parts, using our new modules, which provide a layered development system to automatically support api calls from other systems and xml based model templates to describe configuration data.
https://wiki.opnsense.org/index.php/Develop:Creating_the_hello_world_module (https://wiki.opnsense.org/index.php/Develop:Creating_the_hello_world_module)
https://wiki.opnsense.org/index.php/Howto_use_the_API (https://wiki.opnsense.org/index.php/Howto_use_the_API)


Our guidelines somewhat depend of the stage the code is in, when writing new code, all actions should use the api system for actually changing configuration and performing configuration tasks. They should, of course, use the normal PSR coding standards for php code and follow the python peps.

When moving to the legacy part of the system, our goal is to stick as close to PSR1/2 as possible, knowing it will never be perfect.

As soon as we can find some time, we really should add a wiki document for the coding guidelines. (The wiki really needs work, but is not high enough on my todo list at the moment)