-
Notifications
You must be signed in to change notification settings - Fork 2
Contributing to Nitro
Thanks for taking the time to be a part of our team!
Below is a set of guidelines for contributing to Nitro. Once familiar with the rules, head over here for the process.
Discussion on why this structure was chosen
- master
- Pull request needs to be up to date (pass the tests)
- Pull request needs approval from at least 1 reviewer
- feature1
- feature2
- ...
Directly committing to master is not allowed, enforced by branch policy. PRs must pass build tests before merging into master.
We are using Github Actions for CI as described in VSCode documentation. Might switch to TravisCI later.
All rules from the following site with checkmarks are activated.
https://eslint.org/docs/rules/
In order to see the results of a the lint test on the code pushed, make sure that to use the up-to-date ESlint code from the master branch. Then, go to commits and click on the red X or green checkmark that corresponds to the commit. From there, click on the details next to the option that says "ESlint / ESLint (push)". If the submitted code did not pass the linter, then the number of lint errors will also be displayed in that option.
The first line of a CL description should be a short summary of specifically what is being done by the CL, followed by a blank line. Try to keep your first line short, focused, and to the point. The rest of the description should be informative. It might include a brief description of the problem that’s being solved, and why this is the best approach.
The rules for commit linting are listed in the following site:
https://www.npmjs.com/package/@commitlint/config-conventional
An example of a commit message that passes this linter would be "fix: some message", where "fix" is a type listed in the following array: [ 'build', 'ci', 'chore', 'docs', 'feat', 'fix', 'perf', 'refactor', 'revert', 'style', 'test' ]
The other checks this linter does are mostly on capitalization and empty commit messages and can be checked for details in the link above.
Developers are required to write unit tests for their javascript additions. QA will write integration tests, and developers will coordinate with QA when their component is finished.
Put the name of the exercise in the subject line of the commit. E.g. hamming: Add test case for strands of unequal length. The subject line should be a one-sentence summary. Any extra detail should be provided in the body of the pull request.
Don't submit unrelated changes in the same pull request.
Don't push lot of small commits that really make up one change, squash your commits. Refer to the guidelines on squashing commits.
Keep PRs small. Generally, this is about 100 LOC or about 5 files. Anything bigger should try to be broken up into more than one PR. Tests are not counted towards line of file counts. Generally, tests should be included in their relevant PR. Any exception to the line count is deleting a file; this could be considered 1 LOC, but still 1 file. For more detail
If refactoring was required to add your change, then put two commits: First the refactoring, then the added behavior. It's fine to put this in the same pull request, unless the refactoring is huge and would make it hard to review both at the same time.
✨ Some useful links to get familiar with Chrome extensions or CI ✨
Chrome Extension Getting Started
Mocha
xvbf-action
Testing example