diff options
Diffstat (limited to 'docs/contributing/development.rst')
-rw-r--r-- | docs/contributing/development.rst | 139 |
1 files changed, 100 insertions, 39 deletions
diff --git a/docs/contributing/development.rst b/docs/contributing/development.rst index 12d706f3..1c257772 100644 --- a/docs/contributing/development.rst +++ b/docs/contributing/development.rst @@ -1,7 +1,8 @@ .. _development: +########### Development -=========== +########### All VyOS source code is hosted on GitHub under the VyOS organization which can be found here: https://github.com/vyos @@ -20,8 +21,8 @@ https://github.com/vyos/vyos-build The README.md file will guide you to use the this top level repository. -Submit a patch --------------- +Submit a Patch +============== Patches are always more then welcome. To have a clean and easy to maintain repository we have some guidelines when working with Git. A clean repository @@ -32,22 +33,26 @@ file(s) history by invoking ``git log path/to/file.txt``. .. _prepare_commit: -Preparing patch/commit -^^^^^^^^^^^^^^^^^^^^^^ +Prepare patch/commit +-------------------- In a big system, such as VyOS, that is comprised of multiple components, it's impossible to keep track of all the changes and bugs/feature requests in one's head. We use a bugtracker known as Phabricator_ for it ("issue tracker" would be a better term, but this one stuck). -The information is used in two ways: +The information is used in three ways: * Keep track of the progress (what we've already done in this branch and what we still need to do). * Prepare release notes for upcoming releases -To make this approach work, every change must be associated with a bug number +* Help future maintainers of VyOS (it could be you!) to find out why certain + things have been changed in the codebase or why certain features have been + added + +To make this approach work, every change must be associated with a task number (prefixed with **T**) and a component. If there is no bug report/feature request for the changes you are going to make, you have to create a Phabricator_ task first. Once there is an entry in Phabricator_, you should reference its id in @@ -60,19 +65,21 @@ If there is no Phabricator_ reference in the commits of your pull request, we have to ask you to ammend the commit message. Otherwise we will have to reject it. -In general, use an editor to create your commit messages rather than passing -them on the command line. The format should be and is inspired by this blog -post: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html +Writing good commit messages +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -* A single, short, summary of the commit (recommended 70 characters or less, - but not exceeding 80 characters) +The format should be and is inspired by: https://git-scm.com/book/ch5-2.html - * Add a prefix of the changed component to your commit headline, e.g. ``snmp: - T1111:`` or ``ethernet: T2222:``. If multiple components are touched by this - commit, you can use multiple prefixes, e.g.: ``snmp: ethernet:`` +* A single, short, summary of the commit (recommended 50 characters or less, + not exceeding 80 characters) containing a prefix of the changed component + and the corresponding Phabricator_ reference e.g. ``snmp: T1111:`` or + ``ethernet: T2222:`` - multiple components could be concatenated as in + ``snmp: ethernet: T3333`` -* Followed by a blank line (this is mandatory - else Git will treat the whole - commit message as the headline only) +* In some contexts, the first line is treated as the subject of an email and + the rest of the text as the body. The blank line separating the summary from + the body is critical (unless you omit the body entirely); tools like rebase + can get confused if you run the two together. * Followed by a message which describes all the details like: @@ -107,8 +114,9 @@ Limits: Please submit your patches using the well-known GitHub pull-request against our repositories found in the VyOS GitHub organisation at https://github.com/vyos -Determining package for a fix -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Determinine source package +-------------------------- Suppose you want to make a change in the webproxy script but yet you do not know which of the many VyOS packages ship this file. You can determine the VyOS @@ -124,8 +132,9 @@ This means the file in question (``/opt/vyatta/sbin/vyatta-update-webproxy.pl``) is located in the ``vyatta-webproxy`` package which can be found here: https://github.com/vyos/vyatta-webproxy -Fork repository to submit a Patch -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Fork Repository and submit Patch +-------------------------------- Forking the repository and submitting a GitHub pull-request is the preferred way of submitting your changes to VyOS. You can fork any VyOS repository to your @@ -160,8 +169,9 @@ record them in your created Git commit: * Submit the patch ``git push`` and create the GitHub pull-request. + Attach patch to Phabricator task -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +-------------------------------- Follow the above steps on how to "Fork repository to submit a Patch". Instead of uploading "pushing" your changes to GitHub you can export the patches/ @@ -171,8 +181,9 @@ commits and send it to maintainers@vyos.net or attach it directly to the bug * Export last commit to patch file: ``git format-patch`` or export the last two commits into its appropriate patch files: ``git format-patch -2`` + Coding Guidelines ------------------ +================= Like any other project we have some small guidelines about our source code, too. The rules we have are not there to punish you - the rules are in place to help @@ -183,8 +194,9 @@ implied logic of any one source file.. Python 3 **shall** be used. How long can we keep Python 2 alive anyway? No considerations for Python 2 compatibility **should** be taken at any time. + Formatting -^^^^^^^^^^ +---------- * Python: Tabs **shall not** be used. Every indentation level should be 4 spaces * XML: Tabs **shall not** be used. Every indentation level should be 2 spaces @@ -194,8 +206,9 @@ Formatting ``au FileType xml setlocal equalprg=xmllint\ --format\ --recover\ -\ 2>/dev/null`` now you can call the linter using ``gg=G`` in command mode. + Text generation -############### +^^^^^^^^^^^^^^^ Template processor **should** be used for generating config files. Built-in string formatting **may** be used for simple line-oriented formats where every @@ -204,8 +217,9 @@ used for structured, multi-line formats such as those used by ISC DHCPd. The default template processor for VyOS code is Jinja2_. + Summary -####### +------- When modifying the source code, remember these rules of the legacy elimination campaign: @@ -214,8 +228,9 @@ campaign: * No old style command definitions * No code incompatible with Python3 + Python ------- +====== The switch to the Python programming language for new code is not merely a change of the language, but a chance to rethink and improve the programming @@ -239,8 +254,9 @@ Please use the following template as good starting point when developing new modules or even rewrite a whole bunch of code in the new style XML/Pyhon interface. -Configuration script structure and behaviour -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Configuration Script Structure and Behaviour +-------------------------------------------- Your configuration script or operation mode script which is also written in Python3 should have a line break on 80 characters. This seems to be a bit odd @@ -360,8 +376,9 @@ For easy orientation we suggest you take a look on the ``ntp.py`` or ``interfaces-bonding.py`` (for tag nodes) implementation. Both files can be found in the vyos-1x_ repository. -XML - CLI ---------- + +XML (used for CLI definitions) +============================== The bash (or better vbash) completion in VyOS is defined in *templates*. Templates are text files (called ``node.def``) stored in a directory tree. The @@ -384,7 +401,6 @@ there is no other format now that would allow this. Besides, a specialized XML editor can alleviate the issue with verbosity. Example: -^^^^^^^^ .. code-block:: xml @@ -466,18 +482,58 @@ Command definitions are purely declarative, and cannot contain any logic. All logic for generating config files for target applications, restarting services and so on is implemented in configuration scripts instead. +GNU Preprocessor +---------------- + +XML interface definition files use the `xml.in` file extension which was +implemented in T1843_. XML interface definitions tend to have a lot of +duplicated code in areas such as: + +* VIF (incl. VIF-S/VIF-C) +* Address +* Description +* Enabled/Disabled + +Instead of supplying all those XML nodes multiple times there are now include +files with predefined features. Brief overview: + +* `IPv4, IPv6 and DHCP(v6)`_ address assignment +* `IPv4, IPv6`_ address assignment +* `VLAN (VIF)`_ definition +* `MAC address`_ assignment + +All interface definition XML input files (.in suffix) will be sent to the GCC +preprocess and the output is stored in the `build/interface-definitions` +folder. The previously mentioned `scripts/build-command-templates` script +operates on the `build/interface-definitions` folder to generate all required +CLI nodes. + +.. code-block:: none + + $ make interface_definitions + install -d -m 0755 build/interface-definitions + install -d -m 0755 build/op-mode-definitions + Generating build/interface-definitions/intel_qat.xml from interface-definitions/intel_qat.xml.in + Generating build/interface-definitions/interfaces-bonding.xml from interface-definitions/interfaces-bonding.xml.in + Generating build/interface-definitions/cron.xml from interface-definitions/cron.xml.in + Generating build/interface-definitions/pppoe-server.xml from interface-definitions/pppoe-server.xml.in + Generating build/interface-definitions/mdns-repeater.xml from interface-definitions/mdns-repeater.xml.in + Generating build/interface-definitions/tftp-server.xml from interface-definitions/tftp-server.xml.in + [...] + + Guidelines -^^^^^^^^^^ +---------- Use of numbers -############## +^^^^^^^^^^^^^^^ Use of numbers in command names **should** be avoided unless a number is a part of a protocol name or similar. Thus, ``protocols ospfv3`` is perfectly fine, but something like ``server-1`` is questionable at best. Help String -########### +^^^^^^^^^^^ To ensure uniform look and feel, and improve readability, we should follow a set of guidelines consistently. @@ -554,7 +610,7 @@ Examples: * Bad: "Disables IPv6 forwarding" Migrating old CLI -^^^^^^^^^^^^^^^^^ +----------------- .. list-table:: :widths: 25 25 50 @@ -618,7 +674,7 @@ Migrating old CLI - All logic should be in the scripts Continous Integration ---------------------- +===================== VyOS makes use of Jenkins_ as our Continous Integration (CI) service. Our CI server is publicly accessible here: https://ci.vyos.net. You can get a brief @@ -636,9 +692,14 @@ to our Debian repository which is used during build time. It is located here: http://dev.packages.vyos.net/repositories/. .. _process: https://blog.vyos.io/vyos-development-digest-10 -.. _VyConf: https://github.com/vyos/vyconf/blob/master/data/schemata -.. _vyos-1x: https://github.com/vyos/vyos-1x/blob/current/schema/ +.. _VyConf: https://github.com/vyos/vyconf/tree/master/data/schemata +.. _vyos-1x: https://github.com/vyos/vyos-1x/tree/current/schema .. _Jinja2: https://jinja.palletsprojects.com/ .. _Phabricator: https://phabricator.vyos.net/ .. _Jenkins: https://jenkins.io/ .. _Dockerhub: https://hub.docker.com/u/vyos/ +.. _T1843: https://phabricator.vyos.net/T1843 +.. _`IPv4, IPv6 and DHCP(v6)`: https://github.com/vyos/vyos-1x/tree/current/interface-definitions/include/address-ipv4-ipv6-dhcp.xml.i +.. _`IPv4, IPv6`: https://github.com/vyos/vyos-1x/tree/current/interface-definitions/include/address-ipv4-ipv6.xml.i +.. _`VLAN (VIF)`: https://github.com/vyos/vyos-1x/tree/current/interface-definitions/include/vif.xml.i +.. _`MAC address`: https://github.com/vyos/vyos-1x/tree/current/interface-definitions/include/interface-mac.xml.i |