3.9 KiB
Contributing to Noms
License
Noms is open source software, licensed under the Apache License, Version 2.0.
Contributing code
Due to legal reasons, all contributors must sign a contributor agreement, either for an individual or corporation, before a pull request can be accepted.
Languages
- Use Go, JS, or Python.
- Shell script is not allowed.
Coding style
- Go uses
gofmt, advisable to hook into your editor - JS follows the Airbnb Style Guide
- Tag PRs with either
toward: #<bug>orfixes: #<bug>to help establish context for why a change is happening - Commit messages follow Chris Beam's awesome commit message style guide
Submitting PRs
We follow a code review protocol dervied from the one that the Chromium team uses:
- Create a GitHub fork of the repo you want to modify (e.g., fork
https://github.com/attic-labs/nomstohttps://github.com/<username>/noms). - Add your own fork as a remote to your github repo:
git remote add <username> https://github.com/<username>/noms. - Push your changes to a branch at your fork:
git push <username> <branch> - Create a PR using the branch you just created. Usually you can do this by just navigating to https://github.com/attic-labs/noms in a browser - GitHub recognizes the new branch and offers to create a PR for you.
- When you're ready for review, use GitHub's assign UI to assign someone to review. Typically nobody will review until you assign someone (because we assume you're still getting it ready for review).
- Reviewer will make comments, then say either 'LGTM' (looks good to me) or 'BTY' (back to you).
- If the reviewer said LGTM, it means it is ready to merge. If you have commit rights to the respository, go ahead and land the PR. Otherwise the reviewer will land it.
- Important: Only squash merges are allowed, because we like each commit in master to be a single logical piece of work. GitHub may generate an odd commit message, so double check before clicking Confirm!
- If the reviewer said BTY, make the requested changes.
- Important: Please make each round of review comments its own commit. This makes it easy for reviewers to see how your PR has evolved in response to feedback.
- Important: Please do not rebase on top of master until the end of the review for the same reason - you're trying to make it easy for the reviewer to see your changes in isolation.
- Comment on the review with 'PTAL' (please take another look) when you are ready for the next round of review comments.
For very trivial fixes that are time-sensitive (e.g., to unbreak the build), we do review-after-land. In that case, assign someone to review the PR, and add the phrase 'TBR' (to be reviewed) to the PR description.
Running the tests
You can use go test command, e.g:
go test $(go list ./... | grep -v /vendor/)should run every test except from vendor packages.
For JS code, We have a python script to run all js tests.
python tools/run-all-js-tests.py
If you have commit rights, Jenkins automatically runs the Go and JS tests on every PR, then every subsequent patch. To ask Jenkins to immediately run, any committer can reply (no quotes) "Jenkins: test this" to your PR.
Perf tests
By default, neither go test nor Jenkins run the perf tests, because they take a while.
To run the tests yourself, use the -perf and -v flag to go test, e.g.:
go test -v ./samples/go/csv/... -perf mem
See https://godoc.org/github.com/attic-labs/noms/go/perf/suite for full documentation and flags.
To ask Jenkins to run the perf tests for you, reply (no quotes) "Jenkins: perf this" to your PR. Your results will be viewable at http://perf.noms.io/?ds=http://demo.noms.io/perf::pr_$your-pull-request-number/csv-import. Again, only a committer can do this.