278 lines
10 KiB
Plaintext
278 lines
10 KiB
Plaintext
|
|
1 Patch guidelines
|
|
|
|
This section lists some guidelines for writing a good patch which is
|
|
more likely to be accepted.
|
|
|
|
Any new features or large scale work should first be discussed on the
|
|
evolution-hackers list first. This will ensure the idea fits in the
|
|
direction we wish to take Evolution, and also that the effort is not
|
|
duplicated. See section 3 for details on the mailing lists.
|
|
|
|
1.1 Patch basics
|
|
|
|
o The patch should apply cleanly at the time it is made.
|
|
|
|
o It must compile once applied.
|
|
|
|
o It must not generate any more compile time warnings than were
|
|
already there. This may be platform dependent so simply do your
|
|
best.
|
|
|
|
o It must conform to C89/C90 (ANSI/ISO C), and build with gcc using
|
|
the default compile flags.
|
|
|
|
The primary trap is 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 '{'.
|
|
|
|
If you wish to ensure the code is C89, try the following.
|
|
|
|
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.
|
|
|
|
[FIXME: Add the same option for Forte here]
|
|
|
|
o It should not add any extra debug printing by default, unless the
|
|
patch is specifically to add extra debug printing.
|
|
|
|
o It should not use any gcc extensions, except where they are properly
|
|
checked for and not used with other compilers. glib provides some
|
|
of these features as portable macros and should be used when they
|
|
cover the required functionality.
|
|
|
|
1.1 GUI changes
|
|
|
|
If the change requires non-trivial user interface changes, then they
|
|
will have to be discussed and approved on the evolution-hackers list
|
|
first. This is highly recommended before embarking on any UI work, or
|
|
large scale work in general. The Gnome HIG document is the place to
|
|
start on any UI changes or additions.
|
|
|
|
1.2 Translated string changes
|
|
|
|
Any changes to translated strings in a stable release must be
|
|
discussed on the hackers list (see section 3), and/or as part of the
|
|
patch submission. There must be very good reasons for changing the
|
|
strings in this case.
|
|
|
|
1.3 Coding style
|
|
|
|
Generally the coding style employed matches the "Linux Kernel" style,
|
|
that is, basically K&R style indenting with 8 space tabs. Tabs should
|
|
be used rather than space characters. Reformatting of otherwise
|
|
unchanged code is not acceptable. Editors should have any automatic
|
|
reformatting features disabled.
|
|
|
|
K&R style indenting puts braces on the same line. The opening
|
|
parenthesis of a function call or conditional statement should be on
|
|
the same line as the function. "else" "} else" and "} else {" must
|
|
always occur on lines by themselves.
|
|
|
|
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 only 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 (int frank)
|
|
{
|
|
int a = 1;
|
|
|
|
if (a == frank) {
|
|
a = foo (a);
|
|
} else {
|
|
do {
|
|
a = bob (frank) + a;
|
|
} until (a == frank);
|
|
|
|
frank = a;
|
|
}
|
|
|
|
return (TheType) a;
|
|
}
|
|
|
|
Where there are slight stylistic differences, the style in the
|
|
surrounding code should be followed.
|
|
|
|
1.3.1 Object casts
|
|
|
|
You can either use C style casts, or Gtk style casts. Note that Gtk
|
|
style casts can add significant execution overhead, which is not
|
|
adding any extra checking. e.g. if arguments have already been
|
|
type-checked by preconditions. Putting a space between a cast and a
|
|
variable is optional, but preferred by most of the developers.
|
|
|
|
1.3.2 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 unecessary. Object initialisation and other virtual
|
|
method invocations are considered internal entry points.
|
|
|
|
1.3.3 Line lengths
|
|
|
|
Do not expend effort and resort to unreadable formatting merely to fit
|
|
any long lines into 80 column widths. 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 now all uses high resolution displays, and not
|
|
circa-80's VT100 terminals!
|
|
|
|
On the other hand, lines should generally not exceed 100 characters,
|
|
and absolutely not exceed 160 characters. If your tab nesting is too
|
|
deep you probably have a poor design that needs rethinking.
|
|
|
|
1.4 Design
|
|
|
|
This is a tricky issue to document, but the design of new code should
|
|
`fit' with the existing design of the relevent 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 'i' for a loop variable, if that's all
|
|
it is, don't use 'the_current_index'. etc.
|
|
|
|
If in doubt, ask on the lists.
|
|
|
|
2. Patch submission guidelines
|
|
|
|
This section outlines procedures that should be followed when
|
|
submitting patches to evolution, via the evolution-patches mailing
|
|
list.
|
|
|
|
You must subcribe to the list at
|
|
`http://lists.ximian.com/mailman/listinfo/evolution-patches' before you
|
|
can submit patches to it.
|
|
|
|
Also note that if you attach a patch to a bug report, it should always
|
|
be sent to the list for attention.
|
|
|
|
Any non-trival patches (patches of more than 1 or 2 changed lines in
|
|
more than 5 isolated locations) also require copyright assignment.
|
|
See http://developer.ximian.com/projects/evolution/copyright.html for
|
|
details.
|
|
|
|
If you follow the guidelines listed here, you should generally expect
|
|
a response within 2 working days. If you re-send the same patch
|
|
repeatedly, you will more likely receive less attention. Do not
|
|
re-send the same patch repeatedly.
|
|
|
|
2.1 Subject Lines
|
|
|
|
If the patch addresses a specific bug in bugzilla.ximian.com, then the
|
|
bug number must be included in the subject line, preferably near the
|
|
beginning of the subject line. A concise summary of the bug(s) being
|
|
addressed, should be the remainder of the subject.
|
|
|
|
It is unnecessary to add "[PATCH]", "patch" or similar to the subject
|
|
line, unless it is being cross-posted to other non-patch lists.
|
|
|
|
It is absolutely unnecessary to add "please consider", "please review",
|
|
or "seeking review", or similar, to the subject line. Please do not do
|
|
this.
|
|
|
|
Where the patch does not address a specific bug number, then the subject
|
|
line should simply be a concise summary of the problem/feature it
|
|
addresses.
|
|
|
|
In all cases the subject line should include the module(s) to which the
|
|
patch applies, and would generally match the component on the bug or
|
|
the top-level module directory (e.g. camel, mail, addressbook, use 'all'
|
|
for more than 3 or 4 modules).
|
|
|
|
2.2 Message Body
|
|
|
|
Patches should be attached as attachments, preferably as a single
|
|
diff, when possible, and the changes are related. The diff must be in
|
|
unified diff format, "-up" is a suitable argument to give to "cvs
|
|
diff" (-p may be dropped if not supported by your diff). If you have
|
|
added files, then -N should also be used, but if you are using cvs,
|
|
"cvs add" is needed, and requires write access to the repository.
|
|
|
|
If the patch does not address a specific bug, then the patch email
|
|
should describe which feature or problem it addresses. If it does
|
|
address a specific bug, then further explanation beyond the bug
|
|
commentary is optional, although often convenient.
|
|
|
|
It would also be helpful to summarise the module to which it applies
|
|
in the message body.
|
|
|
|
In all cases you should include which branch, or branches, the patch
|
|
is intended to apply to. If this is not given it will be assumed to
|
|
be the trunk (HEAD), and such patches will and must not be applied to
|
|
any stable branch without further approval.
|
|
|
|
2.3 ChangeLogs
|
|
|
|
All patches must include appropriate ChangeLog diff's, to the
|
|
appropriate ChangeLog(s) for the given change (emacs will automatically
|
|
find the correct one, and format the entry appropriately). All but
|
|
the most trivial of patches will not be considered or discussed
|
|
without this. It is ok to contain extra ChangeLog entries for other
|
|
pending patches, but they should not be excessively long - it isn't
|
|
that hard to isolate patch diffs. If the patch addresses a bug in
|
|
bugzilla.ximian.com, then the ChangeLog entry must include some
|
|
reference to that bug number (either the number, or #number, or 'bug
|
|
xxx'). If it addresses a bug in another bug system, it must also
|
|
indicate which bug system ('gnome bugzilla' 'red-hat bugzilla', etc).
|
|
|
|
2.4 Stable branches
|
|
|
|
Generally, any patch to the stable branch from non-core developers
|
|
must address a specific bug in bugzilla.ximian.com. The patch should
|
|
also be attached to the bug in question, with the keyword 'patch' set
|
|
on the bug report. The patch email must identify which stable branch
|
|
and version it is to apply to.
|
|
|
|
3 Mailing lists
|
|
|
|
3.1 Evolution Hackers
|
|
|
|
If you wish to discuss patches before they are submitted, or ideas
|
|
before you start to work on them, do it on the evolution-hackers list,
|
|
which may be subscribed and viewed at
|
|
`http://lists.ximian.com/mailman/listinfo/evolution-hackers'.
|
|
|
|
This is a low-volume list (5-10 posts per day on average).
|
|
|
|
Some patches may be discussed here to get a wider audience, although
|
|
once a patch has been made it should generally be discussed on
|
|
evolution-patches.
|
|
|
|
Feature requests, bug reports, and other user related discussions,
|
|
without the intention to write code to address them, will be ignored.
|
|
|
|
3.2 Evolution Patches
|
|
|
|
The patch submission list evolution-patches may be subscribed and
|
|
viewed at
|
|
`http://lists.ximian.com/mailman/listinfo/evolution-patches'. Once a
|
|
patch has been written, it may be submitted here for discussion, as
|
|
well as final approval.
|
|
|
|
Any non-patch related postings to this list will be ignored.
|