Evolution Patch Guidelines

This document lists some guidelines for writing a good patch which is more likely to be accepted.

Any new features or large scale work should be discussed within a dedicated ticket in GNOME's issue tracker first. This will ensure the idea fits in the direction we wish to take Evolution, and also that the effort is not duplicated.

Patch Basics

The patch must apply cleanly at the time it is made.
It must compile once applied.
It should not generate any more compile time warnings than were already there. This may be platform dependent, so simply do your best on this one.
It must conform to the C89/C90 standard (ANSI/ISO C), and build with GCC using the default compile flags. The trick here is that that in C99 you may define variables anywhere in the code, in C89 they must be declared in a declaration block which follows any block start '{'.
- Note: If you wish to ensure the code is C89, follow these tips from the GCC manual page:
  - To select this standard in GCC, use one of the options '-ansi', '-std=c89' or '-std=iso9899:1990'; to obtain all the diagnostics required by the standard, you should also specify '-pedantic'
  You may actually have to use '-std=gnu89' if libraries have taken advantage of GCC extensions and where not compiled similarly, as the above options will disable all GNU extensions.
Your patch should not add any extra debug printing unless absolutely necessary.
Do not use any GCC extensions, except where they are properly checked for and not used with other compilers. The glib library provides some of these features as portable macros and should be used when they cover the required functionality.

Coding style

Generally the coding style employed matches the "Linux Kernel" style, though not exactly the same. In other words, we use what is essentially a K&R style indenting with 8 space tabs. Tabs should be used rather than space characters. Do not submit patches which merely reformat otherwise unchanged code. To prevent your editor from doing that accidentally, turn off the automatic reformatting features in your editor.

K&R style indenting puts braces on the same line with the code, not on their own. The opening parenthesis of a function call or conditional statement should be on the same line as the function. These items should always appear on lines by themselves:

    else
    } else
    } else {

A single blank line should follow { } blocks (if not immediately followed by the close of another block), and conditional statements, and be used to separate logical groups of statements in the same block.

A single blank line should separate functions and other structures at the top level of the file (i.e. outside functions). The same rule applies to variable declarations at the start of a block.

An example of the most-developer-preferred formatting:

TheType
the_function (gint frank)
{
        gint aa = 1;

        if (aa == frank) {
                aa = foo (aa);
        } else {
                do {
                        aa = bob (frank) + aa;
                } until (aa == frank);

                frank = aa;
        }

        return (TheType) aa;
}

Where there are slight stylistic differences, the style in the surrounding code should be followed. These are guidelines; use your own best judgement.

Object Casts

Use GObject style casts for GObject instances, which provides extra safety checks over C style casts.

For example:

GtkWidget *widget = (GtkWidget *) button;  /* unsafe */
GtkWidget *widget = GTK_WIDGET (button);   /* preferred */

Preconditions

External API entry points should have preconditions (g_return_if_fail, etc.), although their use varies from case to case. Internal entry points and/or when you are guaranteed the type has already been checked, are unnecessary. Object initialisation and other virtual method invocations are considered internal entry points.

Line Lengths

Do not expend effort or resort to unreadable formatting merely to fit long lines into 80 columns. We use 8 space tabs, and because of the lack of namespacing other than extending the function name, many of the function and type names are too long for this to be practical. We're not stuck on VT100 terminals any more.

On the other hand, lines should generally not exceed 100 characters, and absolutely not exceed 160 characters. Very deep tab nesting may point to a flawed code design rather than a formatting issue.

Design

This is a tricky issue to document, but the design of new code should 'fit' with the existing design of the relevant module. It should at the very least, be no worse.

Code should not cross existing abstraction boundaries or attempt to remove or work around them, if required the existing design may need adjustment.

Type and method names should follow the existing practice in the surrounding code. Method arguments should follow the same order as related methods, and should use the same names for matching parameters.

Per file, static class globals are ok, true globals may be ok, but should be used sparingly. Use 'ii' for a loop variable, if that's all it is, don't use 'the_current_index'. etc. Use GLib types (gchar, gint, gpointer, etc.) instead of standard types (char, int, void *, etc.).

If in doubt, ask on the list or within the bug report.

Patch Submission Process

Patches should be submitted through GNOME's GitLab instance. Depending which project the patch changes it might be filled for Evolution-Data-Server, Evolution, Evolution-EWS or Evolution-MAPI.

Historically, the Evolution project required the copyright on all non-trivial patches to be assigned to Novell. As of July 2008, this is no longer required. See this announcement for details.

Branch Rebase

It's sometimes required to rebase the branch against updated master, or due to consecutive changes, to squash multiple commits into a single commit and such. Below are command line steps how to do it, kindly provided by Philip Withnall. This is only a suggestion, there are other ways to achieve the same. This is intended to make it easier for the new contributors, not that familiar with git branch rebasing.

   $ git checkout master
   $ git pull --rebase
   $ git checkout the-extra-branch
   $ git rebase -i master
     # edit the rebase queue to squash all patches
     # edit the commit message when the rebase workflow prompts you to
   $ git push --atomic --force-with-lease

Note that the Evolution projects currently prefer merge requests consisting of a single commit, because it's easier for the review and it can eventually avoid waste of time (like when there's discovered a fault in the first commit, which is fixed in the tenth commit).