Switch to common structure and shared resources
This commit is contained in:
112
docs/internal/code_standards.txt
Normal file
112
docs/internal/code_standards.txt
Normal file
@@ -0,0 +1,112 @@
|
||||
Scope
|
||||
|
||||
This document describes code quality standards for the i4x
|
||||
system.
|
||||
|
||||
1. Coding Standards
|
||||
|
||||
Code falls into four categories:
|
||||
|
||||
* 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.
|
||||
|
||||
1.1 Deployed
|
||||
|
||||
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.
|
||||
|
||||
1.2 Production
|
||||
|
||||
All production code must be peer-reviewed. The code must meet the
|
||||
following standards:
|
||||
|
||||
1) Test Suite. Code must have reasonable, although not complete, test
|
||||
coverage.
|
||||
2) Consistent. Code must follow PEP8
|
||||
3) Clean Abstractions.
|
||||
4) Future-Compatible. Code must not be incompatible with the
|
||||
long-term vision of either the codebase or of edX.
|
||||
5) Properly Documented
|
||||
6) Maintainable and deployable
|
||||
7) Robust.
|
||||
|
||||
All code paths must be manually or automatically verified.
|
||||
|
||||
1.3 Scaffolding
|
||||
|
||||
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.
|
||||
2) Consistent. Code must follow PEP8
|
||||
3) Clean abstractions or obvious throw-away code. One of the goals
|
||||
of scaffolding is to define proper abstractions.
|
||||
4) Future-Compatible. Code must not be incompatible with the
|
||||
long-term vision of either the codebase or of edX.
|
||||
5) Somewhat documented
|
||||
6) Unpluggable. There should be a setting to disable scaffolding code.
|
||||
By default, and by policy, it should never be enabled on production
|
||||
servers.
|
||||
7) Purpose. The scaffolding must provide a clean reason for existence
|
||||
(e.g. define a specific interface, etc.)
|
||||
|
||||
1.4 Prototype
|
||||
|
||||
Prototype code should live in a separate branch. It should strive
|
||||
to follow PEP8, be readable, testable, and future-proof, but we have
|
||||
no hard standards.
|
||||
|
||||
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
|
||||
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
|
||||
we would be uncomfortable placing on a server).
|
||||
* Either the requester or the puller takes ownership for guaranteeing
|
||||
that those issues are resolved within a short timeframe.
|
||||
* Both the requester and the puller must be comfortable with it.
|
||||
* Both the requester and the owner must have a history of/ability to
|
||||
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'.
|
||||
|
||||
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.
|
||||
* 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>)
|
||||
Reference in New Issue
Block a user