diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..5f2fb18d84 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,298 @@ +# Contributing to Open MCT Web + +This document describes the process of contributing to Open MCT Web as well +as the standards that will be applied when evaluating contributions. + +Please be aware that additional agreements will be necessary before we can +accept changes from external contributors. + +## Summary + +The short version: + +1. Write your contribution. +2. Make sure your contribution meets code, test, and commit message + standards as described below. +3. Submit a pull request from a topic branch back to `master`. Include a check + list, as described below. (Optionally, assign this to a specific member + for review.) +4. Respond to any discussion. When the reviewer decides it's ready, they + will merge back `master` and fill out their own check list. + +## Contribution Process + +Open MCT Web uses git for software version control, and for branching and +merging. The central repository is at +https://github.com/nasa/openmctweb.git. + +### Roles + +References to roles are made throughout this document. These are not intended +to reflect titles or long-term job assignments; rather, these are used as +descriptors to refer to members of the development team performing tasks in +the check-in process. These roles are: + +* _Author_: The individual who has made changes to files in the software + repository, and wishes to check these in. +* _Reviewer_: The individual who reviews changes to files before they are + checked in. +* _Integrator_: The individual who performs the task of merging these files. + Usually the reviewer. + +### Branching + +Three basic types of branches may be included in the above repository: + +1. Master branch. +2. Topic branches. +3. Developer branches. + +Branches which do not fit into the above categories may be created and used +during the course of development for various reasons, such as large-scale +refactoring of code or implementation of complex features which may cause +instability. In these exceptional cases it is the responsibility of the +developer who initiates the task which motivated this branching to +communicate to the team the role of these branches and any associated +procedures for the duration of their use. + +#### Master Branch + +The role of the `master` branches is to represent the latest +"ready for test" version of the software. Source code on the master +branch has undergone peer review, and will undergo regular automated +testing with notification on failure. Master branches may be unstable +(particularly for recent features), but the intent is for the stability of +any features on master branches to be non-decreasing. It is the shared +responsibility of authors, reviewers, and integrators to ensure this. + +#### Topic Branches + +Topic branches are used by developers to perform and record work on issues. + +Topic branches need not necessarily be stable, even when pushed to the +central repository; in fact, the practice of making incremental commits +while working on an issue and pushing these to the central repository is +encouraged, to avoid lost work and to share work-in-progress. (Small commits +also help isolate changes, which can help in identifying which change +introduced a defect, particularly when that defect went unnoticed for some +time, e.g. using `git bisect`.) + +Topic branches should be named according to their corresponding issue +identifiers, all lower case, without hyphens. (e.g. branch mct9 would refer +to issue #9.) + +In some cases, work on an issue may warrant the use of multiple divergent +branches; for instance, when a developer wants to try more than one solution +and compare them, or when a "dead end" is reached and an initial approach to +resolving an issue needs to be abandoned. In these cases, a short suffix +should be added to the additional branches; this may be simply a single +character (e.g. wtd481b) or, where useful, a descriptive term for what +distinguishes the branches (e.g. wtd481verbose). It is the responsibility of +the author to communicate which branch is intended to be merged to both the +reviewer and the integrator. + +#### Developer Branches + +Developer branches are any branches used for purposes outside of the scope +of the above; e.g. to try things out, or maintain a "my latest stuff" +branch that is not delayed by the review and integration process. These +may be pushed to the central repository, and may follow any naming convention +desired so long as the owner of the branch is identifiable, and so long as +the name chosen could not be mistaken for a topic or master branch. + +### Merging + +When development is complete on an issue, the first step toward merging it +back into the master branch is to file a Pull Request. The contributions +should meet code, test, and commit message standards as described below, +and the pull request should include a completed author checklist, also +as described below. Pull requests may be assigned to specific team +members when appropriate (e.g. to draw to a specific person's attention.) + +Code review should take place using discussion features within the pull +request. When the reviewer is satisfied, they should add a comment to +the pull request containing the reviewer checklist (from below) and complete +the merge back to the master branch. + +## Standards + +Contributions to Open MCT Web are expected to meet the following standards. +In addition, reviewers should use general discretion before accepting +changes. + +### Code Standards + +JavaScript sources in Open MCT Web must satisfy JSLint under its default +settings. This is verified by the command line build. + +#### Code Guidelines + +JavaScript sources in Open MCT Web should: + +* Use four spaces for indentation. Tabs should not be used. +* Include JSDoc for any exposed API (e.g. public methods, constructors.) +* Include non-JSDoc comments as-needed for explaining private variables, + methods, or algorithms when they are non-obvious. +* Define one public class per script, expressed as a constructor function + returned from an AMD-style module. +* Follow “Java-like” naming conventions. These includes: + * Classes should use camel case, first letter capitalized + (e.g. SomeClassName.) + * Methods, variables, fields, and function names should use camel case, + first letter lower-case (e.g. someVariableName.) Constants + (variables or fields which are meant to be declared and initialized + statically, and never changed) should use only capital letters, with + underscores between words (e.g. SOME_CONSTANT.) + * File name should be the name of the exported class, plus a .js extension + (e.g. SomeClassName.js) +* Avoid anonymous functions, except when functions are short (a few lines) + and/or their inclusion makes sense within the flow of the code + (e.g. as arguments to a forEach call.) +* Avoid deep nesting (especially of functions), except where necessary + (e.g. due to closure scope.) +* End with a single new-line character. +* Expose public methods by declaring them on the class's prototype. +* Within a given function's scope, do not mix declarations and imperative + code, and present these in the following order: + * First, variable declarations and initialization. + * Second, function declarations. + * Third, imperative statements. + * Finally, the returned value. + +Deviations from Open MCT Web code style guidelines require two-party agreement, +typically from the author of the change and its reviewer. + +#### Code Example + +```js +/*global define*/ + +/** + * Bundles should declare themselves as namespaces in whichever source + * file is most like the "main point of entry" to the bundle. + * @namespace some/bundle + */ +define( + ['./OtherClass'], + function (OtherClass) { + "use strict"; + + /** + * A summary of how to use this class goes here. + * + * @constructor + * @memberof some/bundle + */ + function ExampleClass() { + } + + // Methods which are not intended for external use should + // not have JSDoc (or should be marked @private) + ExampleClass.prototype.privateMethod = function () { + }; + + /** + * A summary of this method goes here. + * @param {number} n a parameter + * @returns {number} a return value + */ + ExampleClass.prototype.publicMethod = function (n) { + return n * 2; + } + + return ExampleClass; + } +); +``` + +### Test Standards + +Automated testing shall occur whenever changes are merged into the main +development branch and must be confirmed alongside any pull request. + +Automated tests are typically unit tests which exercise individual software +components. Tests are subject to code review along with the actual +implementation, to ensure that tests are applicable and useful. + +Examples of useful tests: +* Tests which replicate bugs (or their root causes) to verify their + resolution. +* Tests which reflect details from software specifications. +* Tests which exercise edge or corner cases among inputs. +* Tests which verify expected interactions with other components in the + system. + +During automated testing, code coverage metrics will be reported. Line +coverage must remain at or above 80%. + +### Commit Message Standards + +Commit messages should: + +* Contain a one-line subject, followed by one line of white space, + followed by one or more descriptive paragraphs, each separated by one + line of white space. +* Contain a short (usually one word) reference to the feature or subsystem + the commit effects, in square brackets, at the start of the subject line + (e.g. `[Documentation] Draft of check-in process`) +* Contain a reference to a relevant issue number in the body of the commit. + * This is important for traceability; while branch names also provide this, + you cannot tell from looking at a commit what branch it was authored on. +* Describe the change that was made, and any useful rationale therefore. + * Comments in code should explain what things do, commit messages describe + how they came to be done that way. +* Provide sufficient information for a reviewer to understand the changes + made and their relationship to previous code. + +Commit messages should not: + +* Exceed 54 characters in length on the subject line. +* Exceed 72 characters in length in the body of the commit. + * Except where necessary to maintain the structure of machine-readable or + machine-generated text (e.g. error messages) + +See [Contributing to a Project](http://git-scm.com/book/ch5-2.html) from +Pro Git by Shawn Chacon and Ben Straub for a bit of the rationale behind +these standards. + +## Issue Reporting + +Issues are tracked at https://github.com/nasa/openmctweb/issues + +Issues should include: + +* A short description of the issue encountered. +* A longer-form description of the issue encountered. When possible, steps to + reproduce the issue. +* When possible, a description of the impact of the issue. What use case does + it impede? +* An assessment of the severity of the issue. + +Issue severity is categorized as follows (in ascending order): + +* _Trivial_: Minimal impact on the usefulness and functionality of the + software; a "nice-to-have." +* _(Unspecified)_: Major loss of functionality or impairment of use. +* _Critical_: Large-scale loss of functionality or impairment of use, + such that remaining utility becomes marginal. +* _Blocker_: Harmful or otherwise unacceptable behavior. Must fix. + +## Check Lists + +The following check lists should be completed and attached to pull requests +when they are filed (author checklist) and when they are merged (reviewer +checklist.) + +### Author Checklist + +1. Changes address original issue? +2. Unit tests included and/or updated with changes? +3. Command line build passes? +4. Expect to pass code review? + +### Reviewer Checklist + +1. Changes appear to address issue? +2. Appropriate unit tests included? +3. Code style and in-line documentation are appropriate? +4. Commit messages meet standards?