See www.zabbix.com for the official Zabbix site.

Docs/specs/development guidelines

From Zabbix.org
Jump to: navigation, search

Version control

Zabbix uses Subversion/SVN for version control.

Branches

  • All changes should be first made in a development branch, which should be created in branches/dev from an original branch branches/3.0 (svn cp svn://svn.zabbix.com/branches/3.0 svn://svn.zabbix.com/branches/dev/ZBX-1234 -m ".......... [ZBX-1234] created new development branch to fix foo").
  • Trivial fixes (e.g., typos) can be made directly in trunk and stable branch without independent testing. Developer is fully responsible for such changes. Translations and tests can also be updated without creating a separate dev branch.
  • All development branches should be named the same as the issue they are solving, including case. For instance, ZBXNEXT-512.
  • Issues referenced in branch names can only be from ZBX, and ZBXNEXT projects.
  • If work on a development branch takes a long time, changes from the source branch should be periodically merged into it (in the dev branch: svn merge svn://svn.zabbix.com/branches/3.0) and conflicts resolved.
  • After work on the development branch has been completed, source branch should be merged into it (in the dev branch: svn merge svn://svn.zabbix.com/branches/3.0) and committed before testing and code review may happen.
  • After successful testing and code review source branch should be merged into the development branch again, then development branch should be reintegrated back into the source branch (in the original branch branches/3.0: svn merge --reintegrate svn://svn.zabbix.com/branches/dev/ZBX-1234) and commited to SVN repository.
  • If required, just commited changes to the original branch should be merged to other branches too, for example to trunk. Revision should be taken from previous commit result: (in branch trunk: svn merge -c 58886 svn://svn.zabbix.com/branches/3.0).
Note: If your svn client is 1.8 or later, do not use --reintegrate option - it has been deprecated and its use is discouraged.


Note: Source branch should be merged into development branch prior to it's reintegration always, even if there are no conflicts (see "Basic Merging" for details).


Note: Periodic merging from the source branch and merging from the source branch prior the testing must happen because:
a) it is better to discover and easier to solve conflicts as soon as possible and in smaller batches;
b) when testing, it is not worth wasting time on a development branch which is outdated, compared to the current source branch;
c) changes in the source branch may affect the functionality that is tested in the development branch


  • If there are any conflicts when merging into development branch, a commit message should indicate that.
  • If there are any conflicts when merging into trunk, someone else should review or test the merged changes.
  • Development branches should be removed immediately after they have been reintegrated (svn rm svn://svn.zabbix.com/branches/dev/ZBX-1234 -m ".......... [ZBX-1234] removing merged development branch").
  • Merge commits also should have functional commit messages, "merged revision n" is not sufficient.
  • Use svn mv always to preserve modification history, never delete/add files when moving them.

Commits

  • Changes should not be pooled in huge commits - instead, multiple smaller commits (with related changes limited to a single commit) are preferred.
    • Code formatting changes should never be pooled together with other changes.
  • All svn commits should include a short description of that commit - what was changed in the commit, not what is some future goal of that particular development branch. It should be possible for others to understand the nature of changes without looking at the diff. For example, if some error was caused by off-by-one, commit message should list both what problem it fixes and what was the cause for it.
  • Commit message format for direct changes:
 COMPONENTS [ISSUE] description

An example:

 A.F.I...S. [ZBX-7105] fixed performance of housekeeper; improved indexes for table events; fixed SQL statements to use the new indexes
  • All svn commits should reference at least one JIRA issue so that the commits are visible on the tracker. If a change does not have an open issue on the tracker (e.g., a typo), one of generic issues (TODO: should link to these) should be used.
  • All svn commits that fix development bugs (TODO: should link to description of these) should include a list of development bugs fixed.
  • Issues referenced in svn commits can only be from ZBX and ZBXNEXT projects.
  • When fixing regressions, we should always find the commit(s) that broke that particular feature, so that we know what else could have been broken.

SVN properties

  • Subversion property svn:ignore should contain commonly generated files (for example, configure, all .mo files etc)
  • Property svn:executable should only be set on files that really should be executable
  • Property svn:eol-style should be set to native for all plaintext files
  • Property svn:mime-type should be correctly set depending on real file type and only for files which are not detected as "text" by "file --mime-type" command in Linux

To automate this for new files, svn:auto-props property on repository root is configured to these values (as result of ZBX-10418 discussion):

# svn pg svn:auto-props svn://svn.zabbix.com
*.ac = svn:eol-style=native
*.am = svn:eol-style=native
*.bat = svn:eol-style=native
*.c = svn:eol-style=native
*.conf = svn:eol-style=native
*.cpp = svn:eol-style=native
*.css = svn:eol-style=native
*.example = svn:eol-style=native
*.html = svn:eol-style=native
*.inc = svn:eol-style=native
*.init = svn:eol-style=native
*.java = svn:eol-style=native
*.h = svn:eol-style=native
*.js = svn:eol-style=native
*.m4 = svn:eol-style=native
*.man = svn:eol-style=native
*.mc = svn:eol-style=native
*.php = svn:eol-style=native
*.pl = svn:eol-style=native; svn:executable
*.po = svn:eol-style=native
*.rc = svn:eol-style=native
*.rng = svn:eol-style=native
*.scss = svn:eol-style=native
*.sh = svn:eol-style=native; svn:executable
*.sql = svn:eol-style=native
*.template = svn:eol-style=native
*.tmpl = svn:eol-style=native
*.txt = svn:eol-style=native
*.xml = svn:eol-style=native
.htaccess = svn:eol-style=native
ChangeLog* = svn:eol-style=native
README* = svn:eol-style=native
*.dll = svn:mime-type=application/x-dosexec
*.exe = svn:mime-type=application/x-dosexec
*.jar = svn:mime-type=application/java-archive
*.lib = svn:mime-type=application/x-archive
*.gif = svn:mime-type=image/gif
*.ico = svn:mime-type=image/x-icon
*.jpg = svn:mime-type=image/jpeg
*.png = svn:mime-type=image/png
*.svg = svn:mime-type=image/svg+xml
*.ttf = svn:mime-type=application/x-font-ttf
*.wav = svn:mime-type=audio/x-wav
Note: If for the same file mask a svn property defined at both levels (repository and client configuration), then for the property repository's settings have priority over client's settings. If different properties defined at different levels for the same file mask, then all properties will be applied together. Taking that into account it's recommended to keep svn client config file unconfigured for auto properties or configure file masks with values as listed above.

Filetype specific notes

SVG

Files that have been edited with Inkscape may contain sensitive information (inkscape:export-filename attribute may contain full path exposing directory structure, username etc - see Inkscape bugreport) or useless information in attributes inkscape:export-xdpi and inkscape:export-ydpi. These attributes should be removed before committing svg files.

In the Zabbix repository, there's misc/images/strip_inkscape_attributes.xslt file that allows to easily strip those attributes.

Issue tracker

Zabbix uses Jira for issue tracking - https://support.zabbix.com/.

Working with Jira

Creating an issue

  • The ZBX and ZBXNEXT projects are for user visible bug fixes and enhancements.
  • ZBX must be used for bugs only and ZBXNEXT for everything else.
  • When testing issues, usually it should be noted which version or revision was used and whether the issue was reproducible (still there) or not reproducible (works as expected).
  • "Fix Version/s" field is used for two purposes:
    • It may be used to denote when the issue is planned to be fixed - that would be used for open issues with an unreleased version set here,
    • When an issue is fixed (and closed).

Working with an issue

  • If a problem is not reproducible in latest versions, a reasonable effort should be made to discover which version provided the fix (as a minimum, searching the changelog). If version that fixed the issue has been found, it is set as the "Fix Version/s" version. If it can not be found, no version should be set there, and comment should explain the decision. If some older version is not available as "Fix Version/s" to add or delete them (they are "archived" already), ask Richlv to unarchive them temporary.
  • If the problem is not reproducible, but it is not clear whether it has been fixed (for example, it might be suspected that some configuration in the reporter's system exposes the problem), more information should be asked from the reporter and issue should be set to "Need info" state. If the required information is provided, it should be changed away from "Need info" state.
  • Workflow:
    • Person who starts to work on an issue assigns that issue to himself. It is highly recommended to watch the issue. That would allow the developer to respond to comments or reopening of the issue by users later.
    • When a developer starts working on an issue, it is usually known which versions will be fixed. It is suggested to set "Fix Version/s" field to appropriate values as soon as possible - it reduces the possibility of some issue being neglected and makes planning for releases easier.
    • For issues that change at least one frontend file, a sub-issue must be created before marking the issue as resolved. This sub-issue must list all changed, removed and added translatable strings. Special attention should be paid to changed strings instead of simply listing them as removal of one and adding of another string. If no translation string changes happened in this issue, sub-issue must explicitly say that.
    • When development is finished, issue is marked as "Resolved" and a comment with SVN URL to the development branch is added, e. g.: Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-1234
    • "Resolved" issues are processed by tester.
  • Issues found when testing the fix:
    • When tester starts working on an issue he assigns it to himself.
    • A problem or remark during a code review or testing is called sub-issue. For each sub-issue a separate comment is added and is edited when working with it.
    • These comments are always visible to everybody unless there is some reason to keep them visible to developers only (internal information, non-English comments). If possible, maximum amount of information should be in a public comment and the internal information is added as a separate comment with limited access permissions.
    • Sub-issues are enumerated starting from (1).
    • Sub-issue number can be followed by component (optional).
    • Sub-issue comments follow a dialog style. When a developer resolves a sub-issue and commits the fix to SVN, a corresponding comment is edited and the text with a nickname, "RESOLVED" string, commit reference and optional description is appended. When tester considers the sub-issue to be fixed, "CLOSED" string is added. Additional comments are allowed and encouraged.
    • Every sub-issue dialog must end with "CLOSED".
    • There might be a case when closed sub-issue needs to be reopened. In this case use "REOPENED" string.
  • If additional fixes/changes are being done, they are added as sub-issues.

An example of sub-issue dialog:

(1) [GUI] During the fix we broke feature X. This is very sad.

[~developer] Oops, sorry. RESOLVED in r1234

[~tester] Now feature Y is broken. See, if we do A, then hide B by clicking on C, we are
left with D being on top of E. Considering that F should not be there either, this is not good.

[~developer] Ouch, fixed A handling in B. RESOLVED in r1289, r1337

[~tester] CLOSED

An example of sub-issue dialog with 2 testers:

(3) There is no error message when host becomes unavailable.

[~developer] Right. RESOLVED in r2345

[~tester-1] CLOSED

[~tester-2] There is a typo in the error message "canot". REOPENED

[~developer] RESOLVED in r2346

[~tester-2] Perfect. CLOSED
Checking for changed translation strings

An easy way to check for translation string changes is to run update_po.sh in the locale directory. Then a simple script may be used:

#!/bin/bash
lang="${1:-sk}"
svn diff "$lang" | grep -v "^[-+]#:\|^[-+]\"POT-Creation-Date\|[-+]\{3\} $lang/" | grep "^[-+]"

This defaults to checking the sk locale, but you can manually choose any other. It lists changed msgid and msgstr lines (thus also showing whether removed strings were translated in this particular locale).

After checking these changes should be reverted, as this all happens before testing which is likely to cause additional changes.

Testing an issue

Documenting changes

  • If the change requires to update the documentation, the developer marks the issue as "Needs documenting" and assigns it to whoever is responsible for making the changes. The developer may also update the documentation himself.
  • When the changes have been made, the author of the changes sets the status to "Documented" and asks somebody to review.
  • If the issue doesn't need any documentation updates, these steps can be skipped.
  • After the documentation has been updated and reviewed, the issue can be closed.
What should be documented where

Developer should try to think about all the possible implications of a change - what pages change, which labels change, should any screenshots be updated.

Many changes also have to be noted in the "what's new" and "upgrade notes" pages. When deciding what should be documented where, this example might help:

  • "what's new" is to note everything that can be touted as a feature or improvement in any way. Those pages are suggested for users to read, but if they only want to keep on using existing functionality, they can skip it.
  • "upgrade notes" are for noting things that users should know when upgrading, as they change existing functionality or behaviour in some way.

For example, Zabbix 1.8.6 introduced configuration parameter validation - since that version, daemons would not start up if incorrect parameter was encountered. In "what's new" the improvement angle was mentioned - preventing typos. In upgrade notes it was noted that daemons might refuse to start up after the upgrade.

Closing an issue

  • An issue may be closed after all sub-issues have been CLOSED and the required changes to the documentation have been made.
  • When closing the issue, a comment must indicate a revision and Zabbix version(s) that will contain the fix, e. g.:Fixed in pre-1.8.10 r1234, pre-1.9.9 r1235
  • "Fix Version/s" value/s should be reviewed and are set to versions where this issue was fixed. For example, if it was only fixed in trunk, "Fix Version/s" could be 2.1.0. If it was fixed in several branches, all of them should be present in the "Fix Version/s" field (for example, 1.8.16, 2.0.5). Not correct value/s, if any, should be removed.
  • Values used for "Fix Version/s" have to 100% match the Changelog. For example: for changes in trunk branch need to use "2.1.0" and not "2.2.0" if corresponding changes located under "Changes for 2.1.0" header in the Changelog.
  • Issues closed with any Resolution except "Fixed" have to have empty value of Fix Version/s. Without exceptions.
  • Issues closed with the Resolution as "Duplicate" should have linked duplicated issue. It is suggested to add a comment "closed as a duplicate" or similar - it makes it easier to follow who closed the issue.

Components

In Jira, the following components are available. The letter in parenthesis match with letters in changelog entries and svn commit messages.

  • API (A)
  • Documentation (D)
  • Frontend (F)
  • Agent (G)
  • Installation (I)
  • Java gateway (J)
  • Appliance (L)
  • Proxy (P)
  • Server (S)
  • Templates (T)

Documentation includes manpages and changelog fixes. Installation includes initial database schema and data, autotool related changes and other similar issues. Templates deal with the templates that are shipped with Zabbix.

Changelog and commit messages

Each changelog entry and commit message since Zabbix 1.8.10 and 1.9.9 starts with a string that identifies components, changed in that commit or issue. It is a 10 character string (one per component) with component letter for each component that was changed, empty spaces filled with dots. Commit messages includes only things that were changed in the commit itself, changelog entry includes all components that were changed regarding that issue.

Identification letters are always positionally placed like this:

 ADFGIJLPST

For example, API and Frontend would be:

 A.F.......

Server and proxy would be:

 .......PS.

Commit messages that do not actually change anything (for example, creation and deletion of branches) have all components unset:

 ..........

That also includes syncing a dev branch to the source branch (like branches/1.8 or trunk), because that doesn't introduce any changes in the final branch (these changes already exist in branches/1.8 or trunk).

ChangeLog

  • After the component, all changelog entries must have "[ISSUE] ", where ISSUE is issue number on the tracker (like "ZBX-3")
    • When referencing multiple issues in a single changelog entry, a format of [ZBX-1,ZBX-2] should be used.
    • Issues referenced in changelog entries can only be from ZBX and ZBXNEXT projects.
  • All user visible changes between two versions should be listed in the changelog. Thus if a problem appeared after the last release and was fixed before the next, it should not be added to the changelog.
  • Changelog entries should try to describe the fix from user perspective. Instead of "fixed off by one in some_function", "fixed inability to delete last user macro in host properties" would be preferred.
    • Changelog entries should be short summaries of changes done, not exact copies of issue summaries.
  • Changelog entries are separated in new features and improvements, and bugfixes. An example:
 Changes for 2.1.0
 
 New features:
 A.FGI..PS. [ZBXNEXT-1726] added support for optional host metadata to active agent for auto-registration (dimir, Oleg)
 ...G...... [ZBXNEXT-367] added printing Aliases and PerfCounters when agent is run with -p option (aleksej, dimir)
 
 Bug fixes:
 A.F....... [ZBX-6429] fixed Monitoring -> Events filter clearing and events acknowledging (Eduard, Oleg, Toms)
 ....I..... [ZBX-5314] increased size of autoincrement fields for PostgreSQL database (Sasha)
  • If several people worked on the issue, they should be listed in alphabetical order. For instance, see "Eduard, Oleg, Toms" above.
    • No spaces are used in 1.8 branch
    • Starting with 1.9, spaces should be used - for example, (Eduard, Oleg, Toms)
  • Whenever possible, submitters of patches and translations should be acknowledged. This is done by including a string like "; thanks to John Smith" in the changelog entry before the developer's name.
  • All entries should start with a lower case letter and in the past form (e.g. "..F....... [ZBX-3934] fixed ...").
  • Double quotes should be used for all quoting in the log.
  • If a bug fix is not merged to trunk, the corresponding ChangeLog entry should be added to the trunk changelog anyway. If a bug is fixed both in stable branch and trunk, changelog entry should be added both for stable and trunk version. For example, if a bug is fixed only in 2.2.6, changelog entry is added to 2.2 branch changelog and to the trunk changelog - in 2.2.6 section. If a bug is fixed both for 2.2.6 and 2.3.x, changelog entry appears twice in the trunk changelog, both for 2.2.6 and 2.3.5.

API

Deprecating a feature

To preserve backward compatibility when removing an API feature we first deprecate it till the next major release and only then remove it. The following needs to be done when deprecating an API feature:

  • Usage of deprecated API features must be removed from the frontend. To avoid possible errors, the API should trigger PHP notifications when such features are used.
  • The deprecated feature should be noted in the API changelog.
  • A notice must be added in the documentation of the feature. Some examples:

Database

Changing database schema

When there is a need to change database schema and create a database patch, the following changes are necessary:

  1. change database schema in create/src/schema.tmpl:
    +FIELD		|max_columns	|t_integer	|'3'	|NOT NULL	|0
    
  2. add a patch for every little change in create/src/schema.tmpl to src/libs/zbxdbupgrade/dbupgrade_XXXX.c (each patch should be as atomic as possible and make minimal changes to the database), where XXXX denotes the major and minor version the patch is developed for (e.g., src/libs/zbxdbupgrade/dbupgrade_2030.c for 2.3.x development):
    • name function that applies the patch DBpatch_XXXXYYY(), where XXXX is the version specifier described above and YYY is a sequential identifier:
      static int DBpatch_2030118(void) ...
      
    • add code to this function that changes database schema according to changes in #1:
      const ZBX_FIELD	field = {"max_columns", "3", NULL, NULL, 0, ZBX_TYPE_INT, ZBX_NOTNULL, 0}; return DBadd_field("screens_items", &field);
      
    • add this function to the list of patches using DBPATCH_ADD() macro:
      DBPATCH_ADD(2030118, 0, 1)
      
  3. update default row in dbversion table in create/src/schema.tmpl:
    ROW|2030118|2030118
    
  4. change create/src/data.tmpl, if necessary:
    vim create/src/data.tmpl
    
  5. execute the following command to update server's schema code:
    make dbschema
    
  6. make sure both template files can be imported without errors:
    cat database/mysql/{schema,images,data}.sql | mysql -u root zabbix
    
  7. recompile and run the server, and make sure the database is upgraded correctly
    make install; cd ...; sbin/zabbix_server
    
  8. update ZABBIX_DB_VERSION macro in frontends/php/include/defines.inc.php (only if mandatory database version has been changed):
    define('ZABBIX_DB_VERSION', 2030118);
    
  9. update frontends/php/include/schema.inc.php file with the new schema:
    $ php create/bin/gen_php.php && mv create/src/schema.inc.php frontends/php/include/
    

Synchronising translations

See Synchronising translations page for detailed instructions on translation maintenance.