diff options
| -rw-r--r-- | ChangeLog | 4 | ||||
| -rw-r--r-- | HACKING | 279 |
2 files changed, 283 insertions, 0 deletions
@@ -1,3 +1,7 @@ +2003-08-21 Not Zed <NotZed@Ximian.com> + + * HACKING: Wrote one. + 2003-08-20 Bolian Yin <bolian.yin@sun.com> * configure.in: Add a11y checking, and a11y Makefiles @@ -0,0 +1,279 @@ + +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. The list is specifically for the review of patches and subject +lines such as these will lead to your patch submissions being ignored +and/or simply forgotten. + +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. |