Table of Contents

Development process

Development process

Zabbix development guidelines

Version control

Zabbix uses Git with Bitbucket for version control - https://git.zabbix.com/

Git Setup

Git configuration parameters user.name and user.email must be set to values matching user account in JIRA.
Git configuration parameter push.default must be "simple".
Git configuration parameter core.eol must be either unset or "native".

Branches

All changes should be first made in a development (feature) branch, which must be created in feature/* from an original branch, e.g. release/4.2

git checkout -b feature/ZBX-1234-4.2 release/4.2

Trivial fixes (e.g., typos) can be made directly in master and stable branches without independent testing. The developer is fully responsible for such changes. Translations and tests can also be updated without creating a separate feature branch.
All feature branch names must consist of the issue number they are solving (including case) followed by a minus sign and the version suffix. For instance, ZBXNEXT-1234-4.2.
Issues referenced in branch names can only be from ZBX or ZBXNEXT projects.
If work on a feature branch takes a long time then changes from the source branch should be periodically merged into it (in the feature branch):

git pull origin release/4.2 --rebase

and conflicts resolved.

After work on the feature branch has been completed, the source branch must be merged into it (in the feature branch):

git pull origin release/4.2 --rebase

and pushed

git push

before testing and code review may happen.

Once the changes are prepared for review and testing the change author must create a Pull Request (PR) and specify the feature branch and release or master branches as PR parameters.
After successful testing and code review source branch should be merged into the feature branch again, then PR must be merged back into the source branch (in the original branch release/4.2):

 git merge --no-ff feature/ZBX-1234-4.2; git push

If required, the changes to the original branch should be cherry-picked into other relevant branches (e.g. master).

It is important to merge the feature branch into the source by merge commit even if a fast-forward merge is possible.

Periodic merging from the source branch and merging from the source branch prior to 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 a feature branch, a commit message should indicate that.
If there are any conflicts when merging or cherry-picking into master, someone else should review or test the changes.
Feature branches should be removed immediately after they have been merged

git branch -d feature/ZBX-1234; git push origin --delete feature/ZBX-1234

Merge and cherry-pick commits also should have informative commit messages, "cherry-picked commit a1b2c3d" is not sufficient.
It's ok to force-push your changes into the remote feature branch unless someone else is working in the same branch.
It's strictly prohibited to force-push changes into master or release branches.
Main rules to remember when working with Git:
- all changes from feature branches must be merged into master/releases by merge commits
- direct changes in master and releases must be fast-forward
- merged changes in one branch (e.g. release/4.0) must be integrated into other branches (e.g. master, release/4.2) by cherry-picking

Commits

Changes should not be pooled in huge commits - instead, multiple smaller commits (with related changes limited to a single commit) are preferred. Before a commit into a branch, the developer must test the branch on DEV-Jenkins - see Jenkins section for details.
- Code formatting changes should never be pooled together with other changes.
- If commit contains changes in .scss files, make css command should be called, and updated .css files also should be added to commit. .css files should be regenerated together with each merge commit, where any .scss file were modified.
All Git commits should include a short description of what this commit does, not what is some future goal of that particular development branch. Ideally, others should understand the nature of changes without looking at the diff. For example, if some error was caused by off-by-one, the commit message should list both what problem it fixes and what was the cause for it.
Capital casing must be used for acronyms: 'XML', not 'xml'.
Commit message format for direct changes:

COMPONENTS [ISSUES] description

Where ISSUES is a list of comma-separated issues, COMPONENTS is a fixed-size list of components affected by this commit (see #Components).

Examples:

A.F.I...S. [ZBX-7105] fixed performance of housekeeper; improved indexes for table events; fixed SQL statements to use the new indexes
       
       A......... [ZBX-10318,ZBX-10517] added validation of vsize and hsize parameters in screen.create() and screen.update() methods

All Git commits should reference at least one JIRA issue so that the commits are visible in the tracker. If a change does not have an open issue in the tracker (e.g., a typo), one of the generic issues (e.g. ZBXNEXT-686, DEV-137, ZBXNEXT-826) should be used.
All Git commits that fix development bugs should include a list of development bugs fixed.
Issues referenced in Git commits can only be from ZBX or 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.

Jenkins

Before a commit into a branch, the developer must test the branch on DEV-Jenkins and confirm in Jira that all related jobs have been passed.

If changes involve any C code:

backend-agent2
backend-build
backend-cmocka
backend-cmocka-coverage
build-agents
integration

If changes involve upgrade patch logic: in addition to the C-related jobs, run:

dbupgrade

If changes involve any Go code:

backend-agent2
integration

If changes involve any frontend parts:

api
build-templates
frontend
frontend-phpunit
integration
php-syntax-check

Integration team must run:

build-templates

Git ignores, attributes, empty directories

.gitignore in Zabbix root folder contains all common directory and file names that must be ignored by Git (such as .deps, configure, Makefile.in etc)
.gitattributes in Zabbix root folder sets default handling of line endings as text=auto !eol. This is roughly equivalent to svn:eol-style=native property in Subversion unless Git's configuration parameter core.eol is set to values other than native and doesn't match platform's convention (it's ok if core.eol is unset).
.gitattributes also defines files that should be treated as binary:

*.dll binary
*.lib binary
*.exe binary
*.png binary
*.gif binary
*.jpg binary
*.svg binary
*.ico binary
*.jar binary
*.wav binary
*.ttf binary
*.ai binary

Git is technically incapable of storing empty directories. Put .gitignore file into an empty directory to work around this limitation:

# Ignore everything in this directory
       *
       # Except this file
       !.gitignore

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 new features, refactoring, and other non-bug related changes.
Use DEV project for tests (e.g. Cmocka or Selenium tests) and infrastructure-related code changes.
When testing issues, usually it should be noted which version or commit hash 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 the latest versions, a reasonable effort should be made to discover which version provided the fix (as a minimum, searching the changelog). If the 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 the 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 Sasha 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 the 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. This sub-issue should be set to be visible only for Developers.
- When development is finished, issue is marked as "Resolved" and a comment with feature branch name is added: Implemented in [feature/ZBX-1234-6.5|https://git.zabbix.com/projects/ZBX/repos/zabbix/pull-requests/36/overview]
  - Link to Pull request must be used.
  - Comment must be set to Viewable by Developers.
  - "Resolved" issues are processed by the tester.
  - Issues found when testing the fix:
- When the 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 Git, a corresponding comment is edited and the text with a nickname, "RESOLVED" string, commit hash and optional description is appended. When the tester considers the sub-issue to be fixed, the "CLOSED" string is added. Additional comments are allowed and encouraged.
- Every sub-issue dialog must end with "CLOSED".
- There might be a case when a closed sub-issue needs to be reopened. In this case, use the "REOPENED" string.
If additional fixes/changes are being done, they are added as sub-issues.

An example of sub-issue dialog:

(1) [F] During the fix we broke feature X. This is very sad.
       
       [~developer] Oops, sorry. RESOLVED in a1b2c3d
       
       [~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 2a3b4cd, 1b2c3de
       
       [~tester] CLOSED

An example of a sub-issue dialog with 2 testers:

(3) [F] There is no error message when the host becomes unavailable.
       
       [~developer] Right. RESOLVED in 2a3b4cd
       
       [~tester-1] CLOSED
       
       [~tester-2] There is a typo in the error message "cannot". REOPENED
       
       [~developer] RESOLVED in 1b2c3de
       
       [~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}"
       git 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

When the tester accepts the fix he marks an issue as "Tested" and assigns it back to the developer.
After that developer merges development branch to upstream.

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 **

The 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 behavior in some way.

For example, in 4.2.0 Zabbix sender started using all addresses found in ServerActive parameter. This is clearly an improvement over previous versions and should be put in "What's new". But this change requires responses to be presented in a different way by indicating what server the response is from. Users upgrading from earlier versions must be aware of that and make appropriate changes in their scripts if needed. The fact Zabbix sender changed output should be documented in "Upgrade notes".

Closing an issue

An issue may be closed after all sub-issues have been CLOSED or MOVED and the required changes to the documentation have been made.
When closing the issue, a comment must indicate all commits to all Zabbix versions that will contain all the fixes that were made:

    Available in versions:
           pre-6.4.3rc1 - bb3bf236666,8c183cf0d40
           pre-7.0.0alpha1 - f626e219a7d

The length of the hash sum should not exceed 16 characters. Use of hyperlinks is strongly encouraged:

pre-7.0.0alpha1 - [716fab58|https://git.zabbix.com/projects/ZBX/repos/zabbix/commits/716fab5812a60c5a8b860e73c6497090922650b1]

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 Git 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 include 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 any parts from the above (for example, merges, unit test edits) have all components unset:

..........

ChangeLog

After the component, all changelog entries must have "[ISSUE] ", where ISSUE is the issue number on the tracker (like "ZBX-3")
- When referencing multiple issues in a single changelog entry, a format of [ZBX-1,ZBX-2] must 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 must not be added to the changelog.
Changelog entries should describe the fix from a user perspective. Instead of "fixed off by one in some_function", "fixed inability to delete last user macro in host properties" is preferred.
- Changelog entries should be short summaries of changes done, not exact copies of issue summaries.
Changelog entries are separated into new features and improvements, and bugfixes. An example:

Changes for 4.2.0rc2
       
       New features:
       ..F....... [ZBXNEXT-5109] implemented test value saving before test sessions in preprocessing test forms (miks)
       ..F.....S. [ZBXNEXT-1238,ZBXNEXT-4988] added ability to test media types from UI (Ivo, vso)
       ..F....... [ZBXNEXT-5114] improved multiline input control (ashubin)
       ...G...... [ZBXNEXT-5123] added server address and port to the Zabbix sender output (wiper)
       
       Bug fixes:
       ..F....... [ZBX-15931] fixed triggers filter for php 5.4 (talbergs)
       ..F....... [ZBX-15839] fixed broken layout of the breadcrumbs in Safari browser (ashubin)
       ........S. [ZBX-15867] fixed invalid DNS being accepted when receiving discovery contents from Zabbix proxy (vso)
       A......... [ZBX-15821] fixed SQL statement performance used in template.unlink() method (Sasha)

If several people worked on the issue, they must be listed in alphabetical order. For instance, see "Ivo, vso" above.
- No spaces are used in 1.8 branch
- Starting with 1.9, spaces must 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:

....I..... [ZBXNEXT-4841] removed hardcoded location for iconv.h; thanks to Helmut Grohne for the patch (kalimulin)

All entries must start with a lower case letter and in the past form (e.g. "..F....... [ZBX-3934] 'f'ixed ...").
Capital casing must be used for acronyms: 'XML', not 'xml'.
Double quotes must be used for all quoting in the log.
If a bug fix is not merged to master, the corresponding ChangeLog entry must be added to the master ChangeLog anyway. If a bug is fixed both in the stable branch and master, ChangeLog entry must be added both for stable and master version. For example, if a bug is fixed only in 4.2.1, changelog entry is added to release/4.2 branch ChangeLog and to the master ChangeLog - in 4.2.1 section. If a bug is fixed both for 4.2.1 and 4.0.7, changelog entry appears twice in the master ChangeLog, both for 4.2.1 and 4.0.7.
Only release managers can edit ChangeLog manually. Developers add ChangeLog entries by creating separate files in ChangeLog.d directory in feature branches.

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 must be noted in the API changelog.
A notice must be added in the documentation of the feature. Some examples:
- [https://www.zabbix.com/documentation/2.4/manual/api/reference/application/get Deprecating parameters], note that the deprecated parameters are moved to the end of the list;
- [https://www.zabbix.com/documentation/2.4/manual/api/reference/host/getobjects Deprecating a whole method].

Database

Changing database schema

When there is a need to change database schema and create a database patch, the following changes are necessary: # change database schema in create/src/schema.tmpl:

+FIELD       |max_columns    |t_integer  |'3'    |NOT NULL   |0

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)

update default row in dbversion table in create/src/schema.tmpl:

ROW|2030118|2030118

change create/src/data.tmpl, if necessary:

vim create/src/data.tmpl

execute the following command to update server's schema code:

make dbschema

make sure both template files can be imported without errors:

cat database/mysql/{schema,images,data}.sql | mysql -u root zabbix

recompile and run the server, and make sure the database is upgraded correctly

make install; cd ...; sbin/zabbix_server

update the ZABBIX_DB_VERSION macro in ui/include/defines.inc.php (only if mandatory database version has been changed):

define('ZABBIX_DB_VERSION', 2030118);

update the ui/include/schema.inc.php file with the new schema:

$ php create/bin/gen_php.php && mv create/src/schema.inc.php ui/include/

If upgrade patch uses a macro, that macro must be defined separately inside the upgrade patch, when macros specify constants:

#define ZBX_AUDIT_ACTION_ADD            0
       #define ZBX_AUDIT_ACTION_UPDATE         1

But not functions:

define ZBX_DB_CHAR_LENGTH(str)  "char_length(" #str ")"

ZBX_NODATA identifier must be used in the 6th column in the tables definition in the create/src/schema.tmpl file if a table exports data into *.tmpl files. If not, then ZBX_NODATA is not required. Otherwise, ZBX_NODATA is needed to prevent exporting of columns with runtime or sensitive data like passwords, statuses etc.