Hacking OpenDCS

This post documents the development policies used in OpenDCS.

Code Merging

It’s extremely wishful thinking that anyone else is going to want to contribute but should the situation arise, this is the work flow for contributing to the master repository:

File a bug for the given flaw or feature (if one does not already exist) here.
Clone the main repository (if you haven’t already) and initialize with git-flow using git flow init and accepting the defaults.
Start a new feature using git flow feature start <myfeature>, or a new bug fix using git flow hotfix start <myhotfix> (which preferably includes the bug number in its name).
Write code to fix the bug or add the feature. In any case where tests are necessary, the new tests must pass consistently.
All code must follow the project coding style (see below).
The project must remain buildable with all configure options and pass all tests on all platforms.
Publish your branch to a public repository and create a new merge request. Ask for a review.
Rework your code based on suggestions in the review and submit new patches. Return to the review step as necessary.
Upon approval, pull the latest development branch, rebase your branch on it, and push the resulting branch to the public repository. Create a new merge request if the previous is no longer valid following the review cycle.

Coding Style

The coding style to respect in this project is very similar to most Vala projects. In particular, the following rules are largely adapted from the Rygel Coding Style.

4-spaces (and not tabs) for indentation.
'’Prefer’’ lines of less than <= 120 columns
1-space between function name and braces (both calls and signature declarations)
Avoid the use of ‘this’ keyword:
If function signature/call fits in a single line, do not break it into multiple lines.

For methods/functions that take variable argument tuples, all the first elements of tuples are indented normally with the subsequent elements of each tuple indented 4-space more. Like this:

action.get ("ObjectID",
                typeof (string),
                out this.object_id,
            "Filter",
                typeof (string),
                out this.filter,
            "StartingIndex",
                typeof (uint),
                out this.index,
            "RequestedCount",
                typeof (uint),
                out this.requested_count,
            "SortCriteria",
                typeof (string),
                out this.sort_criteria);

'’Prefer’’ descriptive names over abbreviations (unless well-known) & shortening of names. E.g discoverer over disco.
Use ‘var’ in variable declarations wherever possible.
Use ‘as’ to cast wherever possible.
Single statments inside if/else must not be enclosed by ‘{}’.
The more you provide docs in comments, but at the same time avoid over-documenting. Here is an example of useless comment:

// Fetch the document fetch_the_document ();
Each class should go in a separate .vala file & named according to the class in it. E.g Dcs.UI.PolarChart class should go under dcs-ui-polar-chart.vala.
Avoid putting more than 3 ‘using’ statements in each .vala file. If you feel you need to use more, perhaps you should consider refactoring (Move some of the code to a separate class).
Declare the namespace(s) of the class/errordomain with the class/errordomain itself. Like this:
```
private class Dcs.Hello {
...
};
```
Prefer ‘foreach’ over ‘for’.
Add a newline to break the code in logical pieces

Add a newline before each return, throw, break etc. if it is not the only statement in that block

if (condition_applies ()) {
    do_something ();

    return false;
}

if (other_condition_applies ())
    return true;

Except for the break in a switch:

switch (val) {
    case 1:
        debug ("case 1");
        do_one ();
        break;

    default:
        ...
}

If a function returns several equally important values, they should all be given as out arguments. IOW, prefer this:
```
void get_a_and_b (out string a, out string b)
```
rather than the un-even:
```
string get_a_and_b (out b)
```

Additional General Rules

All public symbols which support a Valadoc comment block must have one. This comment block must also be sufficient for gobject-introspection to adequately introspect the symbol for use in other programming languages.
Include a @since statement in the comment block for new symbols.

Vala-specific Rules

Any functions which could block must be async.
Use the language-native Errors for error reporting, not return values.
Take advantage of properties and their automatic notify signals as much as possible (this eliminates the need for most special accessors, mutators, and custom signals and is more conventional).
Class function blocks should be indented like GNU/Telepathy-GLib if/while blocks. It’s arguable that these should be aligned in column 0, as in regular C functions, but it’s too late to change this (as it would make ‘git blame’ useless).
Private and internal class data members should begin with a _ (public data members and local variables should not begin with a _). This is to make non-public data members instantly recognizable as such (which helps readability).
Private and internal class functions should begin with a _ (public functions should not begin with a _). This is to make non-public functions instantly recognizable as such (which helps readability).
Maximize use of the ‘var’ variable type. This shortens the declarations where it’s valid, reducing noise.

Rarely, the use of ‘var’ can obscure the effective type of the variable. In this case, it’s acceptable to provide an explicit type.
Use the ‘unowned’ modifier when it would prevent a non-trivial amount of memory allocation. This is most commonly true for strings, arrays, and non-reference-counted variables.

Do not use ‘unowned’ for reference-counted variables (like objects) since it reduces readability without benefit. And, as of this writing, bgo#638199 forces unowned variables to have an explicit type (preventing the use of ‘var’).
As in most languages, avoid casting. Casting is usually a sign of an error which should be fixed and reduces readability.
Refer to non-local variables and methods with their qualified name. Within a class function, refer to private data members like ‘this._foo’ and foreign package symbols like ‘package_name.symbol’.

This makes scope immediately clear, helping readability.
Use nullable types correctly. This helps readability (and makes the programmer’s intentions clearer about whether a variable may be null). The ultimate goal is for folks to compile correctly with Vala’s strict-non-null mode enabled (https://wiki.gnome.org/Projects/Vala/Tutorial#Strict_Non-Null_Mode). You can compile folks with strict-non-null mode enabled using: make VALAFLAGS=–enable-experimental-non-null
Place the (private) member variable declaration for a variable which backs a property next to the (public) property declaration, rather than at the top of the file. This keeps as much of the code pertaining to a property as possible in one location.
Initialise member variables when declaring them, if possible, rather than in a constructor or construct{} block. If it’s not possible to initialise a member variable at declaration time (e.g. because its value depends on another variable), perform the initialisation in a construct{} block rather than a specific constructor. This means that the initialisation doesn’t have to be copied between multiple alternate constructors.
When iterating over a MultiMap, try to use the map_iterator(). This is more efficient than iterating over the result of get_keys(), then calling get() separately for each key.

Build Health

Before creating a final merge request, the final commit in the series must successfully build and pass ‘make check’ consistently.
TODO insert step here about CI building successfully. Or not.

Profiling OpenDCS

OpenDCS has (actually, will have) various profiling points throughout its startup code, in order to be able to profile the startup process. In order to use this:

Compile dcs with --enable-profiling.
strace -ttt -f -o /tmp/logfile dcs # or some other OpenDCS program
python util/plot-timeline.py -o output.png /tmp/logfile
Examine output.png for obvious problems

This is based on Federico Mena Quintero’s plot-timeline.py. The script has been included in the project repository.