Skip to content

Development Guidelines

Jump to bottom Edit New page
Brian L edited this page Oct 21, 2013 · 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

  • 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

In production/test we use a MySQL database. In development, we use either a SQLite database or a MySQL database. Regardless of the database choice, we use Django-South(What is 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 schema migration APPNAME, 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 should 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 follows from modern graphic cards and browsers. 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 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
  • forms.py
    • Should contain all the code for the custom forms for the application.
    • Reference: Working with Forms
  • tests.py

These are not all the application conventions, but are the main ones. For maintaing 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 the file cannot access the project and neither can the project access it).
  • 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 template 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.).
  • Important: We do not serve static files through Django in production/test, we use Amazon S3, NEVER list the absolute path to a static file unless it is a S3 path.

###URLs URLs should be meaningful. Campaign pages are prefixed with /pinpoint/, but in production, this changes to the name of the campaign. 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 it already exists, use it.

  • We do not write C-style languages.

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

  • 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 80 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 should generally follow this format:

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

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; 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.

Issue Tracking

  • When a PR is opened, the associated story in Planbox should be marked as Blocked with the comment stating "Code Review/Pull Request"; if the Pull Request is accepted, mark the story in Planbox 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 Planbox and tagged with Type: Bug, 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 should be reported in Planbox with Importance: High and completed on a branch off of master with the name hotfix/SUBJECT_OF_FIX.
Add a custom footer
Add a custom sidebar

Clone this wiki locally