summaryrefslogtreecommitdiff
path: root/CONTRIBUTING.md
diff options
context:
space:
mode:
Diffstat (limited to 'CONTRIBUTING.md')
-rw-r--r--CONTRIBUTING.md110
1 files changed, 57 insertions, 53 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index e0de0eb..0ce1e93 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -1,7 +1,7 @@
Contributing to rebar
---------------------
-Before implementing a new feature, please submit a ticket to discuss your plans.
+Before implementing a new feature, please submit a ticket to discuss your plans.
The feature might have been rejected already, or the implementation might already be decided.
See [Community and Resources](README.md#community-and-resources).
@@ -9,95 +9,99 @@ See [Community and Resources](README.md#community-and-resources).
Code style
----------
-The following rules must be followed:
+The following rules apply:
* Do not introduce trailing whitespace
- * Do not mix spaces and tabs
- * Do not introduce lines longer than 80 characters
-
-The following rules should be followed:
+ * We use spaces for indenting only
+ * Try not to introduce lines longer than 80 characters
* Write small functions whenever possible
- * Avoid having too many clauses containing clauses containing clauses.
+ * Avoid having too many clauses containing clauses containing clauses.
Basically, avoid deeply nested functions.
-[erlang-mode (emacs)](http://www.erlang.org/doc/man/erlang.el.html)
-indentation is preferred. This will keep the code base consistent.
-vi users are encouraged to give [Vim emulation](http://emacswiki.org/emacs/Evil) ([more
-info](https://gitorious.org/evil/pages/Home)) a try.
+Follow the indentation style of existing files. The [erlang-mode for
+(emacs)](http://www.erlang.org/doc/man/erlang.el.html) indentation is going to
+always work. Other users may want to use 4-width spaces and make sure things
+align mostly the way they would with Emacs code, or with the rest of the
+project.
+
+Where possible, include type specifications for your code so type analysis
+will be as accurate as possible.
+
+Please add comments around tricky fixes or workarounds so that we can
+easily know why they're there at a glance.
Pull requests and branching
---------------------------
-Use one topic branch per pull request. If you do that, you can add extra commits or fix up
-buggy commits via `git rebase -i`, and update the branch. The updated branch will be
-visible in the same pull request. Therefore, you should not open a new pull request when
-you have to fix your changes.
+All fixes to rebar end up requiring a +1 from one or more of the project's
+maintainers. When opening a pull request, explain what the patch is doing
+and if it makes sense, why the proposed implementation was chosen.
+
+Try to use well-defined commits (one feature per commit) so that reading
+them and testing them is easier for reviewers and while bissecting the code
+base for issues.
+
+During the review process, you may be asked to correct or edit a few things
+before a final rebase to merge things.
-Do not commit to master in your fork.
+Please work in feature branches, and do not commit to `master` in your fork.
Provide a clean branch without merge commits.
Tests
-----
-As a general rule, any behavioral change to rebar requires a test to go with it. If there's
-already a test case, you may have to modify that one. If there isn't a test case or a test
-suite, add a new test case or suite in `inttest/`. [retest](https://github.com/dizzyd/retest) based tests are preferred, but
-we also have EUnit tests in `test/`.
+As a general rule, any behavioral change to rebar requires a test to go with
+it. If there's already a test case, you may have to modify that one. If there
+isn't a test case or a test suite, add a new test case or suite in `test/`.
-Say you've added a new test case in `inttest/erlc`. To only execute the modified suite,
-you would do the following:
-```sh
-# First we build rebar and its deps to also get `deps/retest/retest`
-$ make debug deps
-# Now we can test the modified erlc suite
-$ deps/retest/retest -v inttest/erlc
-```
+To run the tests:
-To test EUnit tests, you would do:
```sh
-$ make debug
-$ ./rebar -v eunit
+$ ./bootstrap
+$ ./rebar3 ct
```
-You can also run `make test` to execute both EUnit and [retest](https://github.com/dizzyd/retest) tests as `make check` does.
+The rebar3 test suite is written using Common Test. As many of the tests as
+possible are written by using the programmatic rebar3 API rather than
+by running the escriptized project directly. The tests should be restricted
+to their `priv_dir` and leave the system clean after a run.
+
+ If such tests prove hard to write, you can ask for help doing that in your
+pull request.
+
+For tests having a lot to do with I/O and terminal interaction, consider
+adding them to https://github.com/tsloughter/rebar3_tests
+
Credit
------
-To give everyone proper credit in addition to the git history, please feel free to append
+To give everyone proper credit in addition to the git history, please feel free to append
your name to `THANKS` in your first contribution.
Committing your changes
-----------------------
-Please ensure that all commits pass all tests, and do not have extra Dialyzer warnings.
-To do that run `make check`. If you didn't build via `make debug` at first, the beam files in
-`ebin/` might be missing debug_info required for [xref](http://www.erlang.org/doc/man/xref.html)
-and [Dialyzer](http://www.erlang.org/doc/man/dialyzer.html), causing a test
-failure.
-If that happens, running `make clean` before running `make check` could solve the problem.
-If you change any of the files with known but safe to ignore Dialyzer warnings, you may
-have to adapt the line number(s) in [dialyzer_reference](dialyzer_reference). If you do that,
-do not remove the
-leading blank line.
+Please ensure that all commits pass all tests, and do not have extra Dialyzer warnings.
+To do that run `./rebar3 ct` and `./rebar3 as dialyze dialyzer`.
#### Structuring your commits
-Fixing a bug is one commit.
-Adding a feature is one commit.
-Adding two features is two commits.
-Two unrelated changes is two commits.
+- Fixing a bug is one commit.
+- Adding a feature is one commit.
+- Adding two features is two commits.
+- Two unrelated changes is two commits (and likely two Pull requests)
-If you fix a (buggy) commit, squash (`git rebase -i`) the changes as a fixup commit into
+If you fix a (buggy) commit, squash (`git rebase -i`) the changes as a fixup commit into
the original commit.
#### Writing Commit Messages
-It's important to write a proper commit title and description. The commit title must be
-at most 50 characters; it is the first line of the commit text. The second line of the
-commit text must be left blank. The third line and beyond is the commit message. You
-should write a commit message. If you do, wrap all lines at 72 characters. You should
-explain what the commit does, what references you used, and any other information
+It's important to write a proper commit title and description. The commit title must be
+at most 50 characters; it is the first line of the commit text. The second line of the
+commit text must be left blank. The third line and beyond is the commit message. You
+should write a commit message. If you do, wrap all lines at 72 characters. You should
+explain what the commit does, what references you used, and any other information
that helps understanding your changes.
Basically, structure your commit message like this: