Code Standards

These documents are very good starting points:

Code

Important

We are going to start by aiming for 70% test coverage for an app and 50% for a project. Useful to bear these in mind Changing the Metrics Conversation and The many flaws of test coverage

Add the following to requirements/local.txt:

pytest-cov
pytest-flakes
pytest-pep8

Add configuration for flakes e.g in setup.cfg:

[tool:pytest]
addopts= --ds=settings.dev_test --cov-report html --reuse-db --fail-on-template-vars
norecursedirs = .git venv-* src
# 1. migrations always import models
# 2. custom settings files e.g. 'dev_patrick.py' do 'from .base import *'
# 3. 'test_view_perm.py' py.test fixtures conflict with pyflakes
flakes-ignore =
    block/migrations/* UnusedImport
    example_block/dev_*.py ImportStarUsed
    test_view_perm.py UnusedImport RedefinedWhileUnused

Important

Change block to the correct path for your app or project.

Warning

DJANGO_SETTINGS_MODULE was being ignored. To fix, add addopts= --ds=settings.dev_test --cov-report html... to addopts and remove DJANGO_SETTINGS_MODULE.

To check the code:

py.test --flakes
py.test --pep8

Release

GIT Commit Messages

From http://chris.beams.io/posts/git-commit/

The seven rules of a great git commit message:

  1. Separate subject from body with a blank line
  2. Limit the subject line to 50 characters
  3. Capitalize the subject line
  4. Do not end the subject line with a period
  5. Use the imperative mood in the subject line
  6. Wrap the body at 72 characters
  7. Use the body to explain what and why vs. how

Tip

And my favourite piece of advice:

A properly formed git commit subject line should always be able to complete the following sentence:

If applied, this commit will your subject line here

Documentation

From PEP 257 - Docstring Conventions - Multi-line Docstrings:

def complex(real=0.0, imag=0.0):
    """Form a complex number.

    Keyword arguments:
    real -- the real part (default 0.0)
    imag -- the imaginary part (default 0.0)
    """
    if imag == 0.0 and real == 0.0:
        return complex_zero

I think I agree with most of the examples in this document: misc/pep8_cheat.pdf.

I also like some of the ideas in Elements of Python Style. We can watch and see if it becomes accepted.

Sphinx

We have two standard folders for Sphinx documentation:

  1. docs-kb, for internal documentation.
  2. docs-user, for documentation written for the client

Note

This documentation should not include secret information.

Model

The order of model inner classes and standard methods should be as follows (they are not all required):

  • All database fields
  • Custom manager attributes
  • class Meta
  • def __unicode__()
  • def __str__()
  • def save()
  • def get_absolute_url()
  • Any custom methods

Delete

Use a boolean field e.g:

deleted = models.BooleanField(default=False)
# optional date and user fields
date_deleted = models.DateTimeField(blank=True, null=True)
user_deleted = models.ForeignKey(
    settings.AUTH_USER_MODEL, blank=True, null=True
)

With an option set_deleted method:

def set_deleted(self, user):
    self.deleted = True
    self.date_deleted = timezone.now()
    self.user_deleted = user
    self.save()

File and Image Field

For a FileField or ImageField, then set the upload_to path to reflect the app and model name e.g. for the document field in the Attachment model in the mail app:

document = models.FileField(upload_to='mail/attachment/')

Testing

Continuous Integration

Create a .gitlab-ci.yml file in the root of the project e.g: https://gitlab.com/kb/contact/blob/master/.gitlab-ci.yml

Create a requirements/ci.txt file e.g: https://gitlab.com/kb/contact/blob/master/requirements/ci.txt

Tip

To make access easier for public repositories, use the GIT https URL rather than a path to the folder e.g: -e git+https://gitlab.com/kb/login.git#egg=login

Check the project setup.cfg file to make sure src is included in the norecursedirs section e.g: https://gitlab.com/kb/contact/blob/master/setup.cfg:

norecursedirs = .git angular venv-* src

Commit and push to GitLab.

In GitLab, for the project, Settings, Project Visibility, Repository, Pipelines

  • Select Only team members.
  • Settings, CI/CD Pipelines
  • Enable the runner you need e.g. ci.hatherleigh.info.
  • Disable shared runners for this project.
  • Set Test coverage parsing to \d+\%\s*$ (copy the pytest-cov (Python) example).

For email notifications for the project, Settings, Integrations, Pipelines emails. Tick Active, add Recipients and Add pusher

Note

Test settings on Builds emails threw an error last time I tried, but the email still sends OK.

Pipelines… create…

Factories

Model factories should create the minimum required to construct a valid object e.g. a product will probably need to create a product category, but a contact will not need to fill in the date of birth.

Note

I am not 100% sure about this… but I am sure a factory which does more than it needs to will make it feel like magic is going on and cause confusion.

Mock

We use the unittest.mock module: https://docs.python.org/3/library/unittest.mock.html

When we next do HTTP/API testing, then checkout https://github.com/patrys/httmock

Model

Create a DjangoModelFactory for the model using Factory Boy and test the following (these are a common source of hard to diagnose issues):

  • ordering
  • str

To mock a model manager:

from unittest import mock

with mock.patch('editor.models.CatArticle.objects') as m:
    m.return_value = {}

URL

From Coding Conventions:

url(regex=r'^$',
    view=views.poll_list,
    name='poll_list',
),

the preferred and wonderfully explicit Jacob Kaplan-Moss / Frank Wiles pattern

Note

Probably best to use the actual view class rather than just the name, using view='polls.views.standard.poll_list',, makes it harder to debug on errors.