Skip to content

Submitting Pull Requests

Ben Cipollini edited this page Mar 17, 2014 · 10 revisions

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]

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.

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.

Documentation

For all new features and any major changes (including class or function additions) for bugfixes, we require documentation to be submitted along with code changes.

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

We use docstrings & comments for each of the following

  • New modules / apps: docstring in the module's __init__.py, explaining the high-level need, design, and dependencies.
  • New files: docstring at the top of the file defining what lives inside, and any overall design.
  • New functions/classes: docstring for each
  • Inline: as needed

Here's an example of the standard docstring for a public function:

def public_fn_with_googley_docstring(name, state=None):
    """This function does something.

    Args:
       name (str):  The name to use.

    Kwargs:
       state (bool): Current state to be in.

    Returns:
       int.  The return code::

          0 -- Success!
          1 -- No good.
          2 -- Try again.

    Raises:
       AttributeError, KeyError

    A really great idea.  A way you might use me is

    >>> print public_fn_with_googley_docstring(name='foo', state=None)
    0

    BTW, this always returns 0.  **NEVER** use with :class:`MyPublicClass`.

    """
    return 0

Testing

We require unit tests for all new features, selenium-based in-browser tests for front-end testing, and ecosystem tests to tests changes to KA Lite's interaction with our central data repository.

Clone this wiki locally