GPII Technical Standards

From wiki.gpii
Jump to: navigation, search

Overview

The GPII's technical standards provide a series of guidelines that all committers and project contributors mutually follow and respect as a community. They are intended to help make our work easier to follow, more transparent, and accessible to those outside the community. They also help to ensure the stability, robustness, code quality, and long-term viability of the GPII technologies. GPII committers are expected to follow these conventions, to help review others' code, encourage adoption of the conventions, and contribute to the refinement of these guidelines over time.

Communication

Public communication about new features, code improvements, bug fixes, and roadmaps is a responsibility of all committers and contributors. Before you start coding, talk about your plans on the appropriate mailing list and stay in touch through the development process to solicit advice, help, and clarification.

Tracking Issues

All tasks, big or small, should be associated with a JIRA ticket. 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. Single-line JIRA descriptions are not sufficient. If you have a large task that will involve a number of substantial commits, consider breaking it up into subtasks.

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.

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.

To initiate a code review the code to be reviewed must be in its own feature branch named after the issue in Jira, e'g GPII-17. The last commit comment will also be the Issue number followed by a colon, then comment. The pull request comment is in the same format and the pull request is made from the feature branch to master. See this example.

The Code Always Works

Integration is everyone's responsibility: the GPII is a large and distributed system, yet each component needs to work in harmony with the others. 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. Code changes should never break the system. 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.

Commit Logs

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.

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

For example:

GPII-501: Added autocomplete support to the whirlygig.
Refactored the whirlygig to include a new autocomplete strategy based on the wireframes provided by the designers.
(http://linkToWireFrames)
 

Branches

When working with your own fork of the GPII repositories, feature branches should be named after the JIRA number (i.e. "GPII-xyz") that 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.

General Code Quality

All code that is pushed into the project repository should be easy to build, test, and run. It shouldn't be hard-wired to your particular development environment, and should include build scripts and 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 wherever possible. This is especially the case if the intention is to promote this code to GPII core repositories.

Code that is pushed into core repositories should not include debug statements (e.g. console.log), commented-out code, or other informal testing artifacts.

Code Quality Checklist

All pull requests submitted to the GPII core repositories should take this code quality checklist into account:

  1. Code should never contain any duplicated sections (cut 'n' paste), with respect to the whole codebase and certainly not amongst any piece of new work.
  2. All new functionality should be accompanied by comprehensive unit and acceptance tests that pass.
  3. Code should be localizable. At a minimum all human readable text should be located in a JSON or other human-editable configuration file.
  4. The code's public API should be clearly commented with JSDoc-style comments.
  5. All core logic must be made available and testable through an API (hint: your event handlers often contain core logic).
  6. ARIA roles, states, and properties must be used where appropriate on HTML-based interfaces.
  7. All functionality must be navigable and usable with the keyboard.
  8. All code must be factored into small, well named things, which are all independently testable in isolation from each other.
  9. Errors must be reported to the user clearly.
  10. All JavaScript files must pass JSHint linting.
  11. All HTML files must pass the W3C validator.
  12. All JSON files must pass JSONLint.
  13. Code should use the GPII Development Platform technologies unless alternatives have been discussed with the community.

Automated Tests

All code submitted to GPII core repositories must be accompanied by a reasonable suite of unit and integration (acceptance) 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

JavaScript

JavaScript is a highly dynamic and loose language, and many common errors are not picked up until run time. In order to avoid errors and common pitfalls in the language, all code should be regularly checked using JSHint. Configuration for JSHint is controlled through .jshintrc and .jshintignore files found in the repo. Our core repositories now include standard grunt tasks for linting - each pull request must pass the linting operated by running grunt lint which will invoke both JSHint and JSON lint for all .js and .json files in the project.

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.

More information about our JavaScript coding conventions can be found on the Coding Conventions page.

C/C++

The GPII tries to avoid native development wherever possible. However, sometimes this is unavoidable, where the platform exposes low-level native functions that are hard or impossible to access via an existing portability layer. Our community expertise in this area is limited and this kind of development should be undertaken in close cooperation with a member of the core architecture team. Some of the Code Quality Checklist points above are applicable to native development, and some are not - in addition, there are some other points which are relevant only for native development.

  1. Code should never contain any duplicated sections (cut 'n' paste), with respect to the whole codebase and certainly not amongst any piece of new work.
  2. All new functionality should be accompanied by comprehensive unit tests that pass.
  3. Code should be localizable. At a minimum all human readable text should be located in a JSON or other human-editable configuration file.
  4. The code's public API should be clearly commented with some variety of Doc-style comments.
  5. All core logic must be made available and testable through an API
  6. All code must be factored into small, well named things, which are all independently testable in isolation from each other.
  7. Errors must be reported to the user clearly.
  8. The code must not allocate fixed-sized buffers which may overflow, or pass raw pointers to dynamically allocated memory. Instead, make use of STL utilities such as std::string and std::auto_ptr.
  9. String values managed by the code should not be encoded as ASCII bytes, but instead in some variety of Unicode (e.g. UTF-8 or UTF-16)
  10. The code should be indented with spaces only, with an indentation level of 4 characters per level.