code_standards.txt 3.96 KB
Newer Older
Piotr Mitros committed
1 2
Scope

3
This document describes code quality standards for the i4x
4 5 6 7 8
system. 

1. Coding Standards

Code falls into four categories:
Piotr Mitros committed
9 10 11 12 13 14 15

* Deployed. Running on a live server. 
* Production. Intended for deployment. 
* Scaffolding. Intended to define interfaces for future work, and
  minimal implementations to support further development. 
* Prototype. Experimental new features. 

16
1.1 Deployed
Piotr Mitros committed
17 18 19 20 21 22 23 24 25

The standards for deployed code are identical to production. In
general, we tend to do either:

1) Perform a final verification QA cycle on changed parts of code
before deploying.
2) Use code on a staging or internal server for a week before
deploying.

26
1.2 Production
Piotr Mitros committed
27

28 29
All production code must be peer-reviewed. The code must meet the
following standards:
Piotr Mitros committed
30 31 32

1) Test Suite. Code must have reasonable, although not complete, test
   coverage.
Piotr Mitros committed
33
2) Consistent. Code must follow PEP8
Piotr Mitros committed
34
3) Clean Abstractions. 
Piotr Mitros committed
35
4) Future-Compatible. Code must not be incompatible with the
Piotr Mitros committed
36 37
   long-term vision of either the codebase or of edX. 
5) Properly Documented
Piotr Mitros committed
38
6) Maintainable and deployable
Piotr Mitros committed
39 40
7) Robust. 

41
All code paths must be manually or automatically verified. 
Piotr Mitros committed
42

43
1.3 Scaffolding
Piotr Mitros committed
44 45 46 47 48 49

All scaffolding code should be peer-reviewed. The code must meet the
following standards:

1) Testable. We do not require test coverage, but we do require the
   code to be structured such that it is possible to build tests.
Piotr Mitros committed
50
2) Consistent. Code must follow PEP8
Piotr Mitros committed
51
3) Clean abstractions or obvious throw-away code. One of the goals 
52
   of scaffolding is to define proper abstractions. 
Piotr Mitros committed
53
4) Future-Compatible. Code must not be incompatible with the
Piotr Mitros committed
54 55
   long-term vision of either the codebase or of edX. 
5) Somewhat documented
Piotr Mitros committed
56 57 58
6) Unpluggable. There should be a setting to disable scaffolding code. 
   By default, and by policy, it should never be enabled on production 
   servers.
59 60
7) Purpose. The scaffolding must provide a clean reason for existence
   (e.g. define a specific interface, etc.)
Piotr Mitros committed
61

62
1.4 Prototype
Piotr Mitros committed
63

64
Prototype code should live in a separate branch. It should strive
Piotr Mitros committed
65 66
to follow PEP8, be readable, testable, and future-proof, but we have
no hard standards. 
67 68 69 70 71 72 73 74 75 76 77

2. Process Standards

* Code should be integrated in small pull requests. Large commits
  should be broken down into small commits for integration.
* Every piece of production and deployed code must be reviewed prior
  to integration.
* Anyone on the edX team competent to review a piece of code may
  review it (this may change as the team grows). 
* Each contributor is responsible for finding a person to review their
  code. If it is not clear to the contributor who is appropriate, each
Piotr Mitros committed
78 79 80 81 82 83 84 85 86
  project has an owner who is the default go-to. 

2.1 Rapid pull

Unmerged code can lead to merge conflicts, and slow down
development. We have an experimental procedure for handling rapid
pulls and merges. To qualify:

* A piece of code must only have minor issues remaining (nothing which
87
  we would be uncomfortable placing on a server).  
Piotr Mitros committed
88
* Either the requester or the puller takes ownership for guaranteeing
89
  that those issues are resolved within a short timeframe.
Piotr Mitros committed
90
* Both the requester and the puller must be comfortable with it. 
Piotr Mitros committed
91
* Both the requester and the owner must have a history of/ability to 
92 93 94 95 96 97 98 99 100 101 102
  resolve remaining issues quickly. 

If code qualifies: 
* It can be merged, and repaired in master. 
* The pull message should specify '## pending fixes/OWNER' where ## is
  the pull request number, and OWNER is the owner. 
* All required fixes are documented in github in the (now closed) pull
  request, and should be marked off there when applied (potentially,
  directly to master).
* Once all fixes are applied, the final commit should specify 
  '## closed'. 
Piotr Mitros committed
103 104 105 106 107 108 109

3. Documentation Standards

* Whenever possible, documentation should live in code. 
* When impossible, it should live in the github repo. 
* Discussion should live on github, Basecamp or Pivotal, depending on
  context.
110 111 112
* Notes for later fixes should in general be put into Pivotal as stories.
  If they are left in the code, they should be prefixed by
  # TODO (<name>)