GPII Technical Standards
All tasks, big or small, should be associated with a JIRA issue. Bug reports and issue descriptions should be meaningful and should elaborate the task, bug, or feature in a way that can be understood by other contributors, code reviewers, and the community. Opaque or general descriptions should be avoided. If you have a large task that will involve a number of substantial commits, consider breaking it up into subtasks.
All commits logs should include, at minimum, the following information:
- A reference to the JIRA issue this commit applies to (at the beginning of the first line)
- A short and meaningful summary of the commit, on the first line
- A meaningful commit log describing the contents of the change
GPII's Github repositories are integrated with our JIRA issue tracker, allowing us to easily cross-reference specific code commits with issues in JIRA. In order to enable this integration, all commit logs should be preceded by the JIRA issue number. JIRA can then automatically associate the change with the bug, so that watchers of an issue can stay apprised of its progress. For example:
GPII-501: Bought Kasper a Carlsberg. Refactored the whirlygig to include a new autocomplete strategy based on the wireframes provided by the designers. (http://linkToWireFrames)
When working with your own fork of the GPII repositories, feature branches should be named after the JIRA issue they are related to. This makes it easier for the community to follow your progress and for a reviewer to correlate a pull request with the related JIRA ticket.
Pull Requests and Code Review
All changes to the GPII core repositories must be submitted as a pull request. Pull requests must be reviewed by another committer, even when submitted by a committer. No direct pushes should be made to the repository without prior review, except in exceptional circumstances.
Code review should be performed in a respectful and welcoming manner using a public forum. Preferably, reviews should be conducted directly on GitHub, but the architecture mailing list may also be used when necessary. Code should be reviewed by a committer who knows the area of the system well. Multiple reviewers are welcome to participate in the process (including non-committers), but advice should be unambiguous and larger-scale discussions/debates should be moved to the mailing list if necessary.
The Code Always Works
As GPII grows and our code is used by a wider community of users, we need to keep code quality high and ensure the integrity of our builds. Our daily builds are also used as a showcase of our work, so pushes to the project repo should never break the build. Please run all unit and acceptance tests before making pull requests to verify that your changes don't cause regressions. If you find you've pushed broken code into the project repo, please fix it expediently or back your code out so that the rest of the community is not affected by a broken build.
General Code Quality
All code that is pushed into the project repository should be easily forked/cloned and built by the community. It shouldn't be hard-wired to your particular development environment, and should include build scripts or quick instructions on how to get the code working.
Experimental or exploratory code should be placed in your own public repo (e.g. your personal GitHub space) or in the GPII Incubator, where the expectations about quality can be more lax to allow experimentation and innovation. You are still encouraged to create meaningful JIRA issues and avoid the use of NOJIRA where possible. This is especially the case if the intention is to promote this code to the project repo.
Code that is pushed into the project repo should not include debug statements (e.g. console.log).
Code Quality Checklist
- Code should never contain any duplicated sections (cut 'n' paste), either with respect to the whole codebase and certainly not amongst any piece of new work.
- All new functionality should be accompanied by comprehensive unit and acceptance tests that pass.
- Code should be localizable. At a minimum all human readable text should be parameterized.
- The code's public API should be clearly commented with JSDoc-style comments.
- All core logic must be made available and testable through an API (hint: your event handlers often contain core logic)
- ARIA roles, states, and properties must be used where appropriate on HTML-based interfaces
- All functionality must be navigable and usable with the keyboard
- All code must be factored into small, well named things, which are all independently testable in isolation from each other
- Errors must be reported to the user clearly
- All HTML files must pass the W3C validator
- All JSON files must pass JSLint
All code submitted to GPII core repositories must be accompanied by a reasonable suite of unit tests. This helps others confirm that the code is working as intended, and allows the community to adopt more agile refactoring techniques while being more confident in our ability to avoid regressions.
Code Conventions and Style
Newer versions of jsHint no longer check whitespace issues. In order to maintain the whitespace checks we use jsHint version 2.4.4 and grunt-contrib-jshint version 0.9.2.
All code submitted to GPII core repositories must pass all lint checks.
Coding style is always a contentious issue, often leading to unproductive "bike shed" debates. Style is a very personal decision. At the same time, style conventions help to make a code base more readable and approachable. In order to avoid arguments about coding style, we've chosen to follow someone else's coding style. Our coding conventions are reflected in our JSHint configuration files.