| -*- mode: text; -*- |
| $QuaggaId: Format:%an, %ai, %h$ $ |
| |
| Contents: |
| |
| * GUIDELINES FOR HACKING ON QUAGGA |
| * COMPILE-TIME CONDITIONAL CODE |
| * COMMIT MESSAGE |
| * HACKING THE BUILD SYSTEM |
| * RELEASE PROCEDURE |
| * SHARED LIBRARY VERSIONING |
| * RELEASE PROCEDURE |
| * TOOL VERSIONS |
| * SHARED LIBRARY VERSIONING |
| * PATCH SUBMISSION |
| * PATCH APPLICATION |
| * STABLE PLATFORMS AND DAEMONS |
| * IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS |
| |
| GUIDELINES FOR HACKING ON QUAGGA |
| |
| [this is a draft in progress] |
| |
| GNU coding standards apply. Indentation follows the result of |
| invoking GNU indent (as of 2.2.8a) with no arguments. Note that this |
| uses tabs instead of spaces where possible for leading whitespace, and |
| assumes that tabs are every 8 columns. Do not attempt to redefine the |
| location of tab stops. Note also that some indentation does not |
| follow GNU style. This is a historical accident, and we generally |
| only clean up whitespace when code is unmaintainable due to whitespace |
| issues, as fewer changes from zebra lead to easier merges. |
| |
| For GNU emacs, use indentation style "gnu". |
| |
| For Vim, use the following lines (note that tabs are at 8, and that |
| softtabstop sets the indentation level): |
| |
| set tabstop=8 |
| set softtabstop=2 |
| set shiftwidth=2 |
| set noexpandtab |
| |
| Be particularly careful not to break platforms/protocols that you |
| cannot test. |
| |
| New code should have good comments, and changes to existing code |
| should in many cases upgrade the comments when necessary for a |
| reviewer to conclude that the change has no unintended consequences. |
| |
| Each file in the Git repository should have a git format-placeholder (like |
| an RCS Id keyword), somewhere very near the top, commented out appropriately |
| for the file type. The placeholder used for Quagga (replacing <dollar> with |
| $) is: |
| |
| $QuaggaId: <dollar>Format:%an, %ai, %h<dollar> $ |
| |
| See line 2 of HACKING for an example; |
| |
| This placeholder string will be expanded out by the 'git archive' commands, |
| wihch is used to generate the tar archives for snapshots and releases. |
| |
| Please document fully the proper use of a new function in the header file |
| in which it is declared. And please consult existing headers for |
| documentation on how to use existing functions. In particular, please consult |
| these header files: |
| |
| lib/log.h logging levels and usage guidance |
| [more to be added] |
| |
| If changing an exported interface, please try to deprecate the interface in |
| an orderly manner. If at all possible, try to retain the old deprecated |
| interface as is, or functionally equivalent. Make a note of when the |
| interface was deprecated and guard the deprecated interface definitions in |
| the header file, ie: |
| |
| /* Deprecated: 20050406 */ |
| #if !defined(QUAGGA_NO_DEPRECATED_INTERFACES) |
| #warning "Using deprecated <libname> (interface(s)|function(s))" |
| ... |
| #endif /* QUAGGA_NO_DEPRECATED_INTERFACES */ |
| |
| To ensure that the core Quagga sources do not use the deprecated interfaces |
| (you should update Quagga sources to use new interfaces, if applicable) |
| while allowing external sources to continue to build. Deprecated interfaces |
| should be excised in the next unstable cycle. |
| |
| Note: If you wish, you can test for GCC and use a function |
| marked with the 'deprecated' attribute. However, you must provide the |
| #warning for other compilers. |
| |
| If changing or removing a command definition, *ensure* that you properly |
| deprecate it - use the _DEPRECATED form of the appropriate DEFUN macro. This |
| is *critical*. Even if the command can no longer function, you *must* still |
| implement it as a do-nothing stub. Failure to follow this causes grief for |
| systems administrators. Deprecated commands should be excised in the next |
| unstable cycle. A list of deprecated commands should be collated for each |
| release. |
| |
| See also below regarding SHARED LIBRARY VERSIONING. |
| |
| COMPILE-TIME CONDITIONAL CODE |
| |
| Please think very carefully before making code conditional at compile time, |
| as it increases maintenance burdens and user confusion. In particular, |
| please avoid gratuitious --enable-.... switches to the configure script - |
| typically code should be good enough to be in Quagga, or it shouldn't be |
| there at all. |
| |
| When code must be compile-time conditional, try have the compiler make it |
| conditional rather than the C pre-processor. I.e. this: |
| |
| if (SOME_SYMBOL) |
| frobnicate(); |
| |
| rather than: |
| |
| #ifdef SOME_SYMBOL |
| frobnicate (); |
| #endif /* SOME_SYMBOL */ |
| |
| Note that the former approach requires ensuring that SOME_SYMBOL will be |
| defined (watch your AC_DEFINEs). |
| |
| COMMIT MESSAGES |
| |
| The commit message should provide: |
| |
| * A suitable one-line summary followed by a blank line as the very |
| first line of the message, in the form: |
| |
| topic: high-level, one line summary |
| |
| Where topic would tend to be name of a subdirectory, and/or daemon, unless |
| there's a more suitable topic (e.g. 'build'). This topic is used to |
| organise change summaries in release announcements. |
| |
| * An optional introduction, discussing the general intent of the change. |
| * A short description of each change made, preferably: |
| * file by file |
| * function by function (use of "ditto", or globs is allowed) |
| |
| to provide a short description of the general intent of the patch, in terms |
| of the problem it solves and how it achieves it, to help reviewers |
| understand. |
| |
| The one-line summary must be limited to 54 characters, and all other |
| lines to 72 characters. |
| |
| The reason for such itemised commit messages is to encourage the author to |
| self-review every line of the patch, as well as provide reviewers an index |
| of which changes are intended, along with a short description for each. |
| Some discretion is obviously required. A C-to-english description is not |
| desireable. For short patches, a per-function/file break-down may be |
| redundant. For longer patches, such a break-down may be essential. |
| |
| An example (where the general discussion is obviously somewhat redundant, |
| given the one-line summary): |
| |
| zebra: Enhance frob FSM to detect loss of frob |
| |
| * (general) Add a new DOWN state to the frob state machine |
| to allow the barinator to detect loss of frob. |
| * frob.h: (struct frob) Add DOWN state flag. |
| * frob.c: (frob_change) set/clear DOWN appropriately on state change. |
| * bar.c: (barinate) Check frob for DOWN state. |
| |
| Note that the commit message format follows git norms, so that "git |
| log --oneline" will have useful output. |
| |
| HACKING THE BUILD SYSTEM |
| |
| If you change or add to the build system (configure.ac, any Makefile.am, |
| etc.), try to check that the following things still work: |
| |
| - make dist |
| - resulting dist tarball builds |
| - out-of-tree builds |
| |
| The quagga.net site relies on make dist to work to generate snapshots. It |
| must work. Common problems are to forget to have some additional file |
| included in the dist, or to have a make rule refer to a source file without |
| using the srcdir variable. |
| |
| RELEASE PROCEDURE |
| |
| * Tag the apppropriate commit with a release tag (follow existing |
| conventions). |
| [This enables recreating the release, and is just good CM practice.] |
| |
| * Create a fresh tar archive of the quagga.net repository, and do a test |
| build: |
| |
| git-clone git:///code.quagga.net/quagga.git quagga |
| git-archive --remote=git://code.quagga.net/quagga.git \ |
| --prefix=quagga-release/ master | tar -xf - |
| cd quagga-release |
| |
| autoreconf -i |
| ./configure |
| make |
| make dist |
| |
| The tarball which 'make dist' creates is the tarball to be released! The |
| git-archive step ensures you're working with code corresponding to that in |
| the official repository, and also carries out keyword expansion. If any |
| errors occur, move tags as needed and start over from the fresh checkouts. |
| Do not append to tarballs, as this has produced non-standards-conforming |
| tarballs in the past. |
| |
| See also: http://wiki.quagga.net/index.php/Main/Processes |
| |
| [TODO: collation of a list of deprecated commands. Possibly can be scripted |
| to extract from vtysh/vtysh_cmd.c] |
| |
| |
| TOOL VERSIONS |
| |
| Require versions of support tools are listed in INSTALL.quagga.txt. |
| Required versions should only be done with due deliberation, as it can |
| cause environments to no longer be able to compile quagga. |
| |
| |
| SHARED LIBRARY VERSIONING |
| |
| [this section is at the moment just gdt's opinion] |
| |
| Quagga builds several shared libaries (lib/libzebra, ospfd/libospf, |
| ospfclient/libsopfapiclient). These may be used by external programs, |
| e.g. a new routing protocol that works with the zebra daemon, or |
| ospfapi clients. The libtool info pages (node Versioning) explain |
| when major and minor version numbers should be changed. These values |
| are set in Makefile.am near the definition of the library. If you |
| make a change that requires changing the shared library version, |
| please update Makefile.am. |
| |
| libospf exports far more than it should, and is needed by ospfapi |
| clients. Only bump libospf for changes to functions for which it is |
| reasonable for a user of ospfapi to call, and please err on the side |
| of not bumping. |
| |
| There is no support intended for installing part of zebra. The core |
| library libzebra and the included daemons should always be built and |
| installed together. |
| |
| |
| PATCH SUBMISSION |
| |
| * Send a clean diff against the 'master' branch of the quagga.git |
| repository, in unified diff format, preferably with the '-p' argument to |
| show C function affected by any chunk, and with the -w and -b arguments to |
| minimise changes. E.g: |
| |
| git diff -up mybranch..remotes/quagga.net/master |
| |
| Or by using git-format-patch. |
| |
| * Not doing so is a definite hindrance to patch application. |
| |
| * Include NEWS entries as appropriate. |
| |
| * Please, please include an appropriate commit message with any emailed |
| patches. Doing so makes it easier to review a patch, and apply it. |
| |
| * Include only one semantic change or group of changes per patch. |
| |
| * Do not make gratuitous changes to whitespace. See the w and b arguments |
| to diff. |
| |
| * State on which platforms and with what daemons the patch has been |
| tested. Understand that if the set of testing locations is small, |
| and the patch might have unforeseen or hard to fix consequences that |
| there may be a call for testers on quagga-dev, and that the patch |
| may be blocked until test results appear. |
| |
| If there are no users for a platform on quagga-dev who are able and |
| willing to verify -current occasionally, that platform may be |
| dropped from the "should be checked" list. |
| |
| |
| PATCH APPLICATION |
| |
| * Only apply patches that meet the submission guidelines. |
| |
| * If the patch might break something, issue a call for testing on the |
| mailinglist. |
| |
| * Give an appropriate commit message (see above), and use the --author |
| argument to git-commit, if required, to ensure proper attribution (you |
| should still be listed as committer) |
| |
| * Immediately after commiting, double-check (with git-log and/or gitk). If |
| there's a small mistake you can easily fix it with 'git commit --amend ..' |
| |
| * By committing a patch, you are responsible for fixing problems |
| resulting from it (or backing it out). |
| |
| |
| STABLE PLATFORMS AND DAEMONS |
| |
| The list of platforms that should be tested follow. This is a list |
| derived from what quagga is thought to run on and for which |
| maintainers can test or there are people on quagga-dev who are able |
| and willing to verify that -current does or does not work correctly. |
| |
| BSD (Free, Net or Open, any platform) # without capabilities |
| GNU/Linux (any distribution, i386) |
| Solaris (strict alignment, any platform) |
| [future: NetBSD/sparc64] |
| |
| The list of daemons that are thought to be stable and that should be |
| tested are: |
| |
| zebra |
| bgpd |
| ripd |
| ospfd |
| ripngd |
| |
| Daemons which are in a testing phase are |
| |
| ospf6d |
| isisd |
| watchquagga |
| |
| |
| IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS |
| |
| The source code of Quagga is based on two vendors: |
| |
| zebra_org (http://www.zebra.org/) |
| isisd_sf (http://isisd.sf.net/) |
| |
| To import code from further sources, e.g. for archival purposes without |
| necessarily having to review and/or fix some changeset, create a branch from |
| 'master': |
| |
| git checkout -b archive/foo master |
| <apply changes> |
| git commit -a "Joe Bar <joe@example.com>" |
| git push quagga archive/foo |
| |
| presuming 'quagga' corresponds to a file in your .git/remotes with |
| configuration for the appropriate Quagga.net repository. |