Skip to content

Development Guidelines

Fraser Harris edited this page Oct 4, 2015 · 19 revisions

Developers spend 3 times more time understanding code than modifying it, and 5 times more time modifying code than writing new code. For this reason, it's important that we make our code as easy to understand as possible by following certain company guidelines.

Code Guidelines

  • Use spaces, not tabs. Specifically, use 4 spaces instead of a tab.

  • Avoid lines longer than 79 characters

  • When using parenthesis () or braces {}, use mitred braces. For example:

      myFunction({        // starts on the same line...
          //something
      });                 // ...ends on its own line.
    
      if (x) {            // starts on the same line...
          //something
      } else {            // ...notice the brace is on the same line here as well.
          //something
      }
    
  • Constants tend to be IN_ALL_CAPS

  • Comments should be meaningful:

    • Good comments should be to the point.
    • Good comments should be accurate, at least at the time you wrote them.
    • Good comments tell the reader what the code does.
    • Good comments tell the reader why this code is necessary.
    • Good comments tell the reader why you wrote the code that way. (Perhaps you wrote hard code because it was necessary -- perhaps you wrote hard code just because you didn't know how to write it in a simpler way -- and this helps the reader know which case that is.)
  • Don't leave commented out code in your commit if the code is already in version control:

    • Comments are for explanations / design decisions
  • Otherwise, we defer to various language standards:

Application Guidelines

###Database Migrations

We use PostgresQL databases, and South for managing migrations. If you write code and make changes that alters an existing model, introduces a new model, or removes an existing model, you will have to create a migration to handle it.

  • South can usually create the migration for you by running python manage.py schemamigration APPNAME --auto, however, there are instances in which South cannot generate the migration in which case you will have to create a migration yourself with a meaningful name.
  • Migrations must be put in the appropriate migrations directory based on the app the migration is for.

###Django The coding style of a Django project/application follows PEP 8 with one exception; lines do not/should not be limited to 79 characters; this convention is not strictly enforced, given that we no longer use 80-character terminals. Full Django Coding Guidelines.

Django has a few application conventions as well:

  • models.py
    • Should contain any code that deals with data access or storage.
    • Should never contain business logic; for example, a Page object must not generate HttpResponses.
    • Should contain the object definitions for the application.
    • Reference: Writing Models
  • admin.py
    • Should contain the code that involves displaying the model in the Django Admin system.
    • Should contain the admin models for the application.
    • Reference: Writing Admin
  • views.py
    • Should contain all code to deal with request handling, form processing and template rendering.
    • This generally involves view functions or view classes.
    • Reference: Writing Views
  • controllers.py
  • forms.py
    • Should contain all the code for the custom forms for the application.
    • Reference: Working with Forms
  • tests/test_*.py

These are not all the application conventions, but are the main ones. For maintaining our Django application, we also have the following guidelines:

  • All folders/subfolders should have a __init__.py file in them if there is a python file inside them (otherwise python files in the same directory cannot be imported).
  • All applications should go in the apps directory.
  • All static files (images, css, js, etc.) should go in a static folder corresponding to the app they interact with.
  • All template files should go in a templates folder corresponding to the app they interact with.
  • Generic static files and fixtures should be placed in the secondfunnel directory with the appropriate subdirectory (static, fixtures, etc.).

###URLs URLs should be meaningful. When adding a url to the urlconf, make sure that the url is meaningful and as short as possible.

  • Good Example: /BLOG/POST244
  • Bad Example: /BLOG/ARCHIVE/POSTS/244

Keep It Simple, Stupid (KISS)

  • If someone else had already written the code, use it.

  • If it already exists in our code, use it.

  • We do not write C-style languages.

    Do not emulate "method overloading" by writing multiple "mostly identical" methods.

  • We (no longer) write Java-style code, either.

    That means code like this is now discouraged in our Python project.

  • Do not give functions arguments that they don't need.

    As an extreme example, function circle(pi, radius, diameter) needs neither pi nor diameter.

  • Do not give classes public attributes we don't support.

  • Factories are a bad idea 90% of the time.

    Think twice when you want to do things like $new_obj = new $type . 'Class';, because you probably don't have to.

Git Guidelines

  • Avoid commit messages longer than 50 characters.

  • Commit messages should be meaningful and reflect the changes.

    • A good commit message: feature/SOMEFEATURE: 887ace4 Replace complicated regex with simple to speed up.
    • A bad commit message: feature/SOMEFEATURE: 887ace4 This is faster.
  • Commit messages shall follow this format:

      {Present tense singular verb} one-liner summarizing the commit
      
      - Change 1
      - Change 2
      - ...
    

from this guide to commit messages

Code Review

  • Pull requests for code reviews follow the same format as git commits. Generally when filing a PR you should:
    • Give the PR an appropriate title and describe why the change is necessary; if the branch name doesn't reflect the purpose of the PR, name it something appropriate.
    • List the changes made and any additional/removed constraints on the project.
  • Code must be reviewed by at least one other person, and ideally including the lead developer.
  • If a PR is incomplete but can be automatically merged, consider closing the PR temporarily, and re-open when appropriate.

Issue Tracking

  • When a PR is opened, the associated task in Asana should indicate that the task is doing worked on by you; if the Pull Request is accepted, mark the story in Asana as Complete, if it is rejected, mark the story as In Progress (unless it is being rejected completely, in which case the story can be deleted).
  • All bugs found in development/production should be logged into Asana, importance should be assigned based on the nature of the bug and where it is found (e.g. A bug in Production that prevents content from being tagged for an upcoming deadline should be given Importance: High while a bug only in development that makes text a little off center would probably be Low or Nice-to-have. When reporting a bug:
    • Specify what the bug is (what does it do? what part of the app does it affect?)
    • Specify what environment(s) it affects (also OS if it only affects a particular OS).
    • Specify how to reproduce the bug.
  • Hotfixes can be completed on a branch off of master with the name hotfix/SUBJECT_OF_FIX.

Clone this wiki locally