So, you’re interested in getting helping out with libtaskotron? Great, this will help get you started on that.
Libtaskotron is written mostly in Python.
The source code for libtaskotron is in a git repository on bitbucket: https://bitbucket.org/fedoraqa/libtaskotron
If you submit patches, please use the process of Submitting code.
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.qadevel.cloud.fedoraproject.org/
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/
There are several tools that, while not required, make the process of developing for libtaskotron significantly easier.
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 yum repository containing Phabricator related packages, run:
sudo curl http://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 yum install arcanist
Arcanist is written in PHP and installing it will pull in several PHP packages as dependencies.
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 yum install gitflow
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.
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.
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.
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.
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
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.
TODO Write documentation on writing directives for libtaskotron
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
The documentation is easy to build once deps are installed:
yum install python-sphinx
To actually build the documentation:
cd docs/
make html
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:
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
To be determined - likely a variation on the docstring convention
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:
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: