Quality Controls
The goals of the project's quality standards are to:
- Enable contributors to be immediately productive
- Reduce unnecessary effort when reviewing contributions
- Promote the stability (i.e. of functionality) and security of the software
We use automated tools to enforce the standards. However, automated tools have limitations and can produce false positives. We should never degrade the quality of the code (e.g., readability) to accommodate the tools. Instead, in these cases, we should document the limitation and ignore the warning.
We use the following tools to enforce style guidelines:
- pep8: for enforcing PEP 8
- Pylint: for enforcing PEP 8 and other style guidlines
- radon: for limiting cyclomatic complexity
- codeclimate-duplication: for detecting code clones
Currently we aren't enforcing style guidelines within the testing modules.
Python contributions should follow the PEP 8 style guidelines, with the following modifications:
- Maximum line length is 119 characters.
The pep8 configuration for the project is maintained in the tox.ini file.
The Pylint configuration for the project is maintained in the pylintrc file. We use the default configuration along with some adjustments to account for the Django framework's behavior. For example, Django's ORM adds an objects member to each model.
You can use the # pylint: disable= directive to disable one or more checks for a statement/code block. If you disable, however, you should include a comment explaining why the check should be disabled. For example:
def __str__(self):
# NOTE: Django's ORM automatically generates a get_xxx_display() member variable
return self.get_value_display() # pylint: disable=no-member
Codacy and Landscape.io are used to run Pylint checks on each commit and PR.
The project enforces a cyclomatic complexity ceiling of 10 inline with line with the C4 Software Technology Reference Guide:
| Cyclomatic Complexity | Risk Evaluation |
|---|---|
| 1-10 | a simple program, without much risk |
| 11-20 | more complex, moderate risk |
| 21-50 | complex, high risk program |
| greater than 50 | untestable program (very high risk) |
Code Climate is used to run the pep8, radon, and codeclimate-duplication checks on each commit and PR. The configuration is maintained in the .codeclimate.yml file.
We currently are not linting the HTML templates and/or HTML produced by Django. See https://github.com/twschiller/open-synthesis/issues/67.
We use the PEP 257 docstring conventions.
[T]esting can be used very effectively to show the presence of bugs but never to show their absence. - Edsger W. Dijkstra
All new code should include tests that:
- Exercise each new line of code
- Have a reasonable set of test oracles (assertions) to determine whether or not the test passed
- When fixing a bug, a test should be introduced that reproduces/detects the bug. That is, the test fails for the previous version of the code, but succeeds in the fixed version
- If you use
#pragma: no coverto exclude a block from coverage reporting, you should include a comment explaining why the code should be excluded
The code coverage configuration is maintained in the .coveragerc file. Travis CI is used to run the test suite for each commit and PR.
We use Django's system check framework to identify common problems. Travis CI is used to run the system checks for each commit and PR.
To run the system checks locally, use:
python manage.py checkWhen deploying to production, uses the --deploy flag to run additional checks for deployment environments. For example, making sure that DEBUG is False:
python manage.py check --deployThe project uses nplusone during testing and when running in DEBUG mode to detect potential N+1 access patterns.
Aside from the checks performed by Django's system checks, we do not currently have any automatic checks for application security. See: https://github.com/twschiller/open-synthesis/issues/63
We do not currently have any automatic checks that run for accessibility. See: https://github.com/twschiller/open-synthesis/issues/56