===========================
Taskotron Development Guide
===========================

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


General Information
===================

Libtaskotron is written mostly in `Python <https://www.python.org/>`_.

Source Code
-----------

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 :ref:`submitting-code`.

.. _taskotron-bugs:

Bugs, Issues and Tasks
----------------------

We use `Phabricator <http://phabricator.org/>`_ 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/


.. _running-tests:

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/

Support tools
-------------

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


.. _installing-arcanist:

Arcanist
^^^^^^^^

`Arcanist <https://secure.phabricator.com/book/phabricator/article/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.


.. _installing-gitflow:

gitflow
^^^^^^^

The `gitflow plugin for git <https://github.com/nvie/gitflow>`_ is another
useful, but not required tool that is available in the Fedora repositories.

To install the gitflow plugin, run::

  sudo yum install gitflow


.. _submitting-code:

Submitting code
---------------

Libtaskotron follows the `gitflow <http://nvie.com/posts/a-successful-git-branching-model/>`_
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 <https://github.com/nvie/gitflow>`_
is recommended as it makes the process significantly easier. See
:ref:`installing-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 :ref:`running-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, :ref:`installing-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 <https://phab.qadevel.cloud.fedoraproject.org/>`_
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 <http://sphinx-doc.org/>`_.

Most of the libtaskotron docs are written in `reStructuredText
<http://docutils.sourceforge.net/rst.html>`_ (reST) and unless otherwise
mentioned, follow the `Python documentation style guide
<https://docs.python.org/devguide/documenting.html#style-guide>`_


Building documentation
----------------------

The documentation is easy to build once deps are installed::

  yum install python-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
<http://sphinx-doc.org/domains.html#info-field-lists>`_


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:

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

      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

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:

  * `reStructuredText Primer <http://sphinx-doc.org/rest.html#rst-primer>`_
  * `Sphinx Markup Constructs <http://sphinx-doc.org/markup/index.html#sphinxmarkup>`_


ReST Headings
^^^^^^^^^^^^^
We use the suggested reST headers as outlined in the `Python documentation style
guide on section headers <https://docs.python.org/devguide/documenting.html#sections>`_
which are:

.. in case the text isn't clear, the following excerpt was taken directly from
   the Python documentation style guide at the link directly above this comment

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