Skip to content

Coding guidelines and conventions

Michael Gallaspy edited this page Feb 8, 2016 · 19 revisions

General Guidelines

Priority on efficiency

KA Lite is designed to function reasonably well on low power devices (such as a Raspberry Pi), meaning we want to avoid doing anything computationally intensive. Also, for the cross-device syncing operations, connection bandwidth and speed are often expensive and slow, so we should always try to minimize the amount of data needing to be transferred.

Avoiding non-python dependencies

Because we want an extremely low friction cross-platform installation process, we only want to depend on Python libraries that are pure Python (no compiled C modules, etc) and cross-platform (i.e., work on both Linux, OSX and Windows). This allows us to fully include any dependencies directly in the codebase, to simplify download and installation.

Soft dependencies on a package with binaries is fine; e.g., for efficiency reasons, the project takes advantage of python-m2crypto if available, but falls back to python-rsa (a pure Python implementation) otherwise.

Git commits

commit messages

Try to use present-tense, imperative-mood sentences for your commit messages, as this is what Git uses for its merge commits by default. For example: "Create a styling table for the frontpage" rather than "creates" or "created".

Python 2.6 restrictions

We have decided to drop support for Python 2.6. Developers can freely use Python 2.7 features, like OrderedDict and set literals.

i18n-everything

Internationalization is core to our project and is sprinkled throughout the code, in terms of features we've created and exposing strings to users.

In order to expose strings to users, please follow the following conventions:

  • In .py files (python code),
    • Make sure to import ugettext (from django.utils.translation import ugettext as _)
    • Wrap any user-facing string with _("String"), with
  • In .html files (Django template files)
    • Make sure to import i18n (`{% load i18n %}' on the second line of the template)
    • Wrap any user-facing string with {% trans 'String' %} or {% blocktrans %}String{% endblocktrans %}
  • In .js files (JavaScript files)
    • Wrap any user-facing string with gettext("String")

Style guides

For code, we generally follow the PEP8 conventions, with a few exceptions/clarifications:

  • Limit line length to 119 characters (PEP8 limits lines to 79 characters, but this can lead to a lot of wrapping)
  • We're somewhat flexible in where we put empty lines; the goal is to use empty lines as punctuation to separate semantic units.

For comments, we follow Google's Python Style Guide, which contain docstring formatting instructions.

  • Read more about expectations related to [documentation and testing](Submitting Pull Requests#Documentation and Testing).

Naming conventions

Besides styles, variable names should be kept consistent.

Paths

  • variables/functions for directory paths: my_varname_dirpath
  • variables/functions for file paths: my_varname_filepath

File pointers:

  • with open('fil.txt') as fp:"

Code Structure Guidelines

App-independence

App creation / editing

We aim to create apps with a very simple dependency structure. This allows for apps to represent features, and therefore to be turned on or off via the INSTALLED_APPS Django variable without causing unexpected failures.

In order to do this, we follow the following conventions:

  • Encapsulation: each app should only use Django-defined globals and globally defined css rules. All other variables, resources, and styles should be defined within the app.
    • No new global Django settings; each app should define and use their own settings in a settings.py file.
    • All app templates should be defined at {app}/templates/{app}/[template].html
    • All static files (images, css, js) should be defined at {app}/static/{app}/[static files]
    • All data files should be defined at {app}/data/{app}/[data files]

Commits

In order to promote reduced inter-app dependencies, we suggest that commits be separated by app. In some cases, this is an absolute necessity (due to the use of git subtree); however, we use this as a project-wide convention.

Imports

Below are the general rules we follow. At the end, find an example that shows each.

We separate our imports into 3 sections:

  1. Libraries that can be installed via requirements.txt (but are currently contained in python-packages)
  2. Django imports
  3. FLE apps (including KA Lite apps as well as apps in other FLE projects, such as securesync and fle-utils).

We have the following conventions:

  • Imports within an app should use relative imports
  • Imports across apps should use absolute import paths

We order our imports by:

  • Putting all import {module} before from {module} import {function} lines.
  • Alphabetizing within import and from sub-sections

Example:

import re  # First section is for external packages that could be installed via requirements.txt
import time
import unittest
from selenium import webdriver  # "from {module} import {function/class}" come after "import module" 
from selenium.common.exceptions import NoSuchElementException
from selenium.webdriver.common.keys import Keys
from selenium.webdriver.support import expected_conditions, ui  # Note the careful alphabetization
from selenium.webdriver.firefox.webdriver import WebDriver

from django.core.urlresolvers import reverse  # Django goes second.  No "import {module} statements, so "from..." appears first.
from django.test import TestCase
from django.utils.translation import ugettext as _

from fle_utils.testing.browser import BrowserTestCase  # All FLE imports also appear here.
from kalite import i18n  # All KA Lite imports are prefaced with kalite.
from kalite.facility.models import Facility, FacilityGroup, FacilityUser
from kalite.main.models import ExerciseLog
from kalite.main.topic_tools import get_exercise_paths

Project-independent Libraries: Python-packages directory

This directory contains code from external projects, and as such should not be modified. Only code within the ka-lite directory should be modified. A few notable exceptions:

  • kalite/distributed/static/khan-exercises - this generally should not be changed
  • python-packages/fle_utils - this can be changed (they are our utilities), but it is best to change in the fle-utils repo directly (adding unit tests to test the functions), then to pull the changes into KA Lite via git subtree pull
  • python-packages/securesync - same as fle_utils above.

CSS: overriding Khan Academy's styling.

The file khan-site.css is from khan-exercises, and we don't want to modify it, in case we want to update it from there in the future. Instead, most CSS styling goes in khan-lite.css, and will override any styles defined in khan-site.css.

For styles that will only ever be used on a single page, they can be defined in a <style> block inside {% block headcss %}. Avoid using inline (tag attribute) styles at all costs.

Django

MVT

Django uses model-view-template pattern, and naturally so do we. Some overall ideas for following MVT is:

  • Fat models and skinny views. Centralize your logic in models, don't disperse it in views.
  • Template logic is template logic! If you want to put a link, use the {% url %} template tage, don't resolve to solutions like putting the link in your context.
  • Furthermore, template logic is also presentation logic. Whether you want 25 or 50 rows displayed on a page is not the view's responsibility but goes in the template. That way, local template overrides can control more, and UI designers can too (without bothering about Python code). If you find yourself putting presentation logic in your view, it's probably because you've forgotten about writing custom template tags :)

On templates

We have a custom static tag that automatically appends a hash to the static url. This makes sure that we reload the assets whenever we update the server through the update process. We strongly recommend that you enable this to make sure that your assets are reloaded on every software update. To do so, just do a {% load kalite_staticfiles %} instead of a {% load staticfiles %} in your templates.

Clone this wiki locally