Taskotron Development Guide

So, you’re interested in helping out with libtaskotron? Great, this will help get you started on that.

General information

Libtaskotron is written mostly in Python.

Source code

The source code for libtaskotron is available at: https://pagure.io/taskotron/libtaskotron

If you submit patches, please use the process of Submitting code.

Bugs, issues and tasks

We use Phabricator to track issues and facilitate code reviews for several projects related to libtaskotron and Fedora QA.

Our phabricator instance can be found at https://phab.qa.fedoraproject.org/

Running unit and functional tests

We place a high value on having decent test coverage for the libtaskotron code. In general, tests are written using pytest <http://pytest.org/> and are broken up into two types:

  • Unit Tests test the core logic of code. They do not touch the filesystem or interact with any network resources. The unit tests should run very quickly
  • Functional Tests are a set of more comprehensive tests which are allowed to touch the filesystem and/or interact with networked resources. Functional tests are often much slower than unit tests but they offer coverage which is not present in the unit tests.

To run the unit tests:

py.test testing/

To run the functional and unit tests:

py.test -F testing/

Continuous integration

Ideally, one should run unit/functional tests before sending code for review. However, it might happen that code that breaks tests gets pushed, for that we have a basic support for running tests after each commit at https://qa.fedoraproject.org/buildmaster/waterfall.

Configuration

The libtaskotron-config package installs config files with default values into /etc/taskotron. If you need to change those default values, you can either change the files in /etc/taskotron or you can create config files inside your checkout with:

cp conf/taskotron.yaml.example conf/taskotron.yaml
cp conf/yumrepoinfo.conf.example conf/yumrepoinfo.conf

The configuration files in conf/ take precedence over anything in /etc, so make sure that you’re editing the correct file if you create local copies.

In the development environment, it’s also useful to have taskotron-generated files automatically cleaned up, so that they don’t occupy disk space in vain. There is a tmpfiles.d template prepared for you, look into conf/tmpfiles.d.

Support tools

There are several tools that, while not required, make the process of developing for libtaskotron significantly easier.

Arcanist

Arcanist is a command line interface to Phabricator which can be used to submit code reviews, download/apply code under review among other useful functions.

As Arcanist is an interface to Phabricator, we strongly recommend that you install it using our packages instead of from the upstream git repos (as described in upstream documentation). That way, there is no question that the Arcanist version used locally is compatible with our Phabricator instance.

To add our dnf repository containing Phabricator related packages, run:

sudo curl https://repos.fedorapeople.org/repos/tflink/phabricator/fedora-phabricator.repo \
-o /etc/yum.repos.d/fedora-phabricator.repo

Once the repository has been configured, install Arcanist using:

sudo dnf -y install arcanist

Arcanist is written in PHP and installing it will pull in several PHP packages as dependencies.

gitflow

The gitflow plugin for git is another useful, but not required tool that is available in the Fedora repositories.

To install the gitflow plugin, run:

sudo dnf -y install gitflow

Submitting code

Libtaskotron follows the gitflow branching model and with the exception of hotfixes, all changes made should be against the develop branch.

While not required, using the gitflow plugin for git is recommended as it makes the process significantly easier. See gitflow for instructions on installing gitflow on Fedora.

Start a new feature

To start work on a new feature, use:

git flow feature start TXXX-short-description

Where TXXX is the issue number in Phabricator and short-description is a short, human understandable description of the change contained in this branch.

In general, short reviews are better than long reviews. If you can, please break up features into chunks that are smaller and easier to manage.

Submitting code for review

Note

Make sure to run all unit and functional tests before submitting code for review. Any code that causes test failure will receive requests to fix the offending code or will be rejected. See Running unit and functional tests for information on running unit and functional tests.

Code reviews are done through Phabricator, not pull requests. While it is possible to submit code for review through the web interface, Arcanist is recommended.

You do not need to push code anywhere public before submitting a review. Unless there is a good reason to do so (and there are very few), pushing a feature branch to origin is frowned on as it makes repository maintenance more difficult for no real benefit.

To submit code for review, make sure that your code has been updated with respect to origin/develop and run the following from your checked-out feature branch:

arc diff develop

The first time that you use Arcanist, it will ask for an API key which can be retrieved from a link contained in that prompt.

Arcanist will create a new code review on our Phabricator instance and prompt you for information about the testing which has been done, a description of the code under review and people to whom the review should be assigned. If you’re not clear on who should review your code, leave the reviewers section blank and someone will either review your code or assign the review task to someone who will.

Updating code reviews

There will often be requests for changes to the code submitted for review. Once the requested changes have been made in your feature branch, commit them and make sure that your branch is still up to date with respect to origin/develop.

To update the existing review, use:

arc diff develop --update DXXX

Where DXXX is the Differential ID assigned to your review when it was originally created.

Pushing code

Note

Changes to git are very difficult to undo or cleanup once they have been pushed to a central repository. If you are not comfortable going through the process listed here, please ask for help.

If you run into any problems before pushing code to origin, please ask for help before pushing.

Once the review has been accepted, the code needs to be merged into develop and pushed to origin.

Make sure that your local develop branch is up-to-date with origin/develop before starting the merge process, else messy commits and merges may ensue. Once develop is up-to-date, the basic workflow to use is:

git checkout feature/TXXX-some-feature
git rebase develop

To merge the code into develop, use one of two commands. If the feature can be reasonably expressed in one commit (most features), use:

git flow feature finish --squash TXXX-some-feature

Else, if the Feature is larger and should cover multiple commits (less common), use:

git flow feature finish TXXX-some-feature

After merging the code, please inspect log messages in case they need to be shortened (Phabricator likes to make long commit messages). Groups of commits should at least have a short description of their content and a link to the revision in differential. Once the feature is ready, push to origin:

git push origin develop

Reviewing code

To review code, use Phabricator’s web interface to submit comments, request changes or accept reviews.

If you want to look at the code under review locally to run tests or test suggestions prior to posting them, use Arcanist to apply the review code.

Make sure that your local repo is at the same base revision as the code under review (usually origin/develop) and run the following command:

arc patch DXXX

Where DXXX is the review id that you’re interested in. Arcanist will grab the code under review and apply it to a local branch named arcpatch-DXXX. You can then look at the code locally or make modifications.

Writing directives

TODO Write documentation on writing directives for libtaskotron

Documentation

The documentation for libtaskotron is made up of several sources which are combined together into a single document using Sphinx.

Most of the libtaskotron docs are written in reStructuredText (reST) and unless otherwise mentioned, follow the Python documentation style guide

Building documentation

The documentation is easy to build once deps are installed. Because system-installed Sphinx is not able to import libraries from virtualenv (bug 1454), it needs to be installed in virtualenv as well:

pip install Sphinx

To actually build the documentation:

cd docs/
make html

Docstrings

Docstrings should be formatted using a reST-like format which Sphinx’s autodoc extension can easily render into HTML.

Sphinx has several built-in info fields which should be used to document function/method arguments and return data.

The following is an excerpt from the Sphinx documentation

Inside Python object description directives, reST field lists with these fields are recognized and formatted nicely:

  • param, parameter, arg, argument, key, keyword: Description of a parameter.
  • type: Type of a parameter.
  • raises, raise, except, exception: That (and when) a specific exception is raised.
  • var, ivar, cvar: Description of a variable.
  • returns, return: Description of the return value.
  • rtype: Return type.

The field names must consist of one of these keywords and an argument (except for returns and rtype, which do not need an argument). This is best explained by an example:

.. py:function:: send_message(sender, recipient, message_body, [priority=1])

   Send a message to a recipient

   :param str sender: The person sending the message
   :param str recipient: The recipient of the message
   :param str message_body: The body of the message
   :param priority: The priority of the message, can be a number 1-5
   :type priority: integer or None
   :return: the message id
   :rtype: int
   :raises ValueError: if the message_body exceeds 160 characters
   :raises TypeError: if the message_body is not a basestring

This will render like this:

send_message(sender, recipient, message_body[, priority=1])

Send a message to a recipient

Parameters:
  • sender (str) – The person sending the message
  • recipient (str) – The recipient of the message
  • message_body (str) – The body of the message
  • priority (integer or None) – The priority of the message, can be a number 1-5
Returns:

the message id

Return type:

int

Raises:
  • ValueError – if the message_body exceeds 160 characters
  • TypeError – if the message_body is not a basestring

It is also possible to combine parameter type and description, if the type is a single word, like this:

:param int priority: The priority of the message, can be a number 1-5

Directive documentation

To be determined - likely a variation on the docstring convention

reST documentation

The majority of libtaskotron’s documentation is in the form of reST files which live in the docs/source directory in the git repository.

For more information see:

ReST headings

We use the suggested reST headers as outlined in the Python documentation style guide on section headers which are:

Section headers are created by underlining (and optionally overlining) the section title with a punctuation character, at least as long as the text:

=================
This is a heading
=================

Normally, there are no heading levels assigned to certain characters as the structure is determined from the succession of headings. However, for the Python documentation, here is a suggested convention:

  • # with overline, for parts
  • * with overline, for chapters
  • =, for sections
  • -, for subsections
  • ^, for subsubsections
  • ", for paragraphs