139b41060SMichael Nitschinger# Contributing
239b41060SMichael Nitschinger
396ea6c27SSimon BasléWe've decided to use Gerrit for our code review system, making it
439b41060SMichael Nitschingereasier for all of us to contribute with code and comments.
539b41060SMichael Nitschinger
696ea6c27SSimon BasléThere are some prerequisites before you can start contributing, so be sure to start with the following:
796ea6c27SSimon Baslé
869436efbSSergey Avseyev  1. Visit [http://review.couchbase.org](http://review.couchbase.org) and "Register" for an account
996ea6c27SSimon Baslé  2. Review [our Contributor Licence Agreement (CLA)](http://review.couchbase.org/static/individual_agreement.html)
1096ea6c27SSimon Baslé  3. Agree to CLA by visiting [http://review.couchbase.org/#/settings/agreements](http://review.couchbase.org/#/settings/agreements)
1196ea6c27SSimon Baslé     Otherwise, you won't be able to push changes to Gerrit (instead getting a "`Upload denied for project`" message).
1239b41060SMichael Nitschinger  4. If you do not receive an email, please contact us
1396ea6c27SSimon Baslé  5. Have a look at current changes in review in the java client area [http://review.couchbase.org/#/q/status:open+project:couchbase-java-client,n,z](http://review.couchbase.org/#/q/status:open+project:couchbase-java-client,n,z)
1439b41060SMichael Nitschinger  6. Join us on IRC at #libcouchbase on Freenode :-)
1539b41060SMichael Nitschinger
1696ea6c27SSimon BasléGeneral information on contributing to Couchbase projects can be found on the [website](http://developer.couchbase.com/open-source-projects#how-to-contribute-code) and in the [wiki](http://www.couchbase.com/wiki/display/couchbase/Contributing+Changes).
1796ea6c27SSimon Baslé
1896ea6c27SSimon Baslé## Building and Testing Locally
1996ea6c27SSimon BasléNote that to build `SNAPSHOT` versions of the `java-client` you also need to build the `core-io` package on which it depends.
2096ea6c27SSimon BasléBoth use maven to package and install. The same process as above applies for the [core-io](https://github.com/couchbase/couchbase-jvm-core) package.
2139b41060SMichael Nitschinger
2296ea6c27SSimon BasléAfter you've checked out both projects (from github) you can build and install them as follows:
2339b41060SMichael Nitschinger
2439b41060SMichael Nitschinger```
2539b41060SMichael Nitschinger┌─[michael@daschlbase]─[~/code/couchbase/couchbase-jvm-core]
2639b41060SMichael Nitschinger└──╼ mvn clean install
2739b41060SMichael Nitschinger**SNIP**
2839b41060SMichael Nitschinger[INFO] --- maven-install-plugin:2.4:install (default-install) @ core-io ---
2939b41060SMichael Nitschinger[INFO] Installing /Users/michaelnitschinger/code/couchbase/couchbase-jvm-core/target/core-io-1.2.1-SNAPSHOT.jar to /Users/michaelnitschinger/.m2/repository/com/couchbase/client/core-io/1.2.1-SNAPSHOT/core-io-1.2.1-SNAPSHOT.jar
3039b41060SMichael Nitschinger[INFO] Installing /Users/michaelnitschinger/code/couchbase/couchbase-jvm-core/dependency-reduced-pom.xml to /Users/michaelnitschinger/.m2/repository/com/couchbase/client/core-io/1.2.1-SNAPSHOT/core-io-1.2.1-SNAPSHOT.pom
3139b41060SMichael Nitschinger[INFO] Installing /Users/michaelnitschinger/code/couchbase/couchbase-jvm-core/target/core-io-1.2.1-SNAPSHOT-sources.jar to /Users/michaelnitschinger/.m2/repository/com/couchbase/client/core-io/1.2.1-SNAPSHOT/core-io-1.2.1-SNAPSHOT-sources.jar
3239b41060SMichael Nitschinger[INFO] Installing /Users/michaelnitschinger/code/couchbase/couchbase-jvm-core/target/core-io-1.2.1-SNAPSHOT-javadoc.jar to /Users/michaelnitschinger/.m2/repository/com/couchbase/client/core-io/1.2.1-SNAPSHOT/core-io-1.2.1-SNAPSHOT-javadoc.jar
3339b41060SMichael Nitschinger[INFO] Installing /Users/michaelnitschinger/code/couchbase/couchbase-jvm-core/target/core-io-1.2.1-SNAPSHOT-sources.jar to /Users/michaelnitschinger/.m2/repository/com/couchbase/client/core-io/1.2.1-SNAPSHOT/core-io-1.2.1-SNAPSHOT-sources.jar
3439b41060SMichael Nitschinger[INFO] ------------------------------------------------------------------------
3539b41060SMichael Nitschinger[INFO] BUILD SUCCESS
3639b41060SMichael Nitschinger[INFO] ------------------------------------------------------------------------
3739b41060SMichael Nitschinger[INFO] Total time: 52.676 s
3839b41060SMichael Nitschinger[INFO] Finished at: 2015-10-12T07:18:50+02:00
3939b41060SMichael Nitschinger[INFO] Final Memory: 36M/337M
4039b41060SMichael Nitschinger[INFO] ------------------------------------------------------------------------
4139b41060SMichael Nitschinger```
4239b41060SMichael Nitschinger
4339b41060SMichael NitschingerNext, the exact steps apply for the  `java-client`.
4439b41060SMichael Nitschinger
4596ea6c27SSimon BasléNote that installing includes running the tests, which require you to run a local Couchbase Server 4.0 or later instance.
4696ea6c27SSimon BasléIf you want to avoid building the tests over and over again, you can add the `-Dmaven.test.skip` flag to the command line.
4796ea6c27SSimon BasléIf you only want to run the unit tests (also no server required for them), use the `-Dunit` flag (recommended over skipping the tests entirely).
4896ea6c27SSimon Baslé
4996ea6c27SSimon Baslé## Preparing for Contribution
5096ea6c27SSimon BasléGerrit needs a little bit of setup in the repository of the project you are planning to contribute to.
5196ea6c27SSimon Baslé
5296ea6c27SSimon Baslé> **Tip**: [Here](https://www.mediawiki.org/wiki/Gerrit/Tutorial) is an extensive tutorial on how to work with git and
5396ea6c27SSimon BasléGerrit. It includes explanations about a [`git-review`](https://github.com/openstack-infra/git-review) CLI tool that can
5496ea6c27SSimon Baslébe used to work with Gerrit instead of the bare git commands described in the following sections.
5596ea6c27SSimon Baslé
5696ea6c27SSimon BasléYou should already have created a Gerrit account and associated it with a SSH key, as well as having signed the Contributor Licence Agreement (CLA, see introduction).
5796ea6c27SSimon Baslé
5896ea6c27SSimon BasléYou should also have performed a `git clone` of the project from Github, so we'll assume you're in the project's directory, `couchbase-java-client/`.
5996ea6c27SSimon Baslé
6096ea6c27SSimon BasléFirst you need to add a remote for Gerrit (replace `YOUR_SSH_USERNAME` with the relevant value from your ssh config):
6196ea6c27SSimon Baslé
6296ea6c27SSimon Baslé```bash
6396ea6c27SSimon Baslégit remote add gerrit ssh://YOUR_SSH_USERNAME@review.couchbase.org:29418/couchbase-java-client.git
6496ea6c27SSimon Baslé```
6596ea6c27SSimon Baslé
6696ea6c27SSimon BasléYou'll also need to install a commit hook script that will generate a new Gerrit Change-Id each time you make a brand new commit:
6796ea6c27SSimon Baslé
6896ea6c27SSimon Baslé```bash
6923f142b3SFelix Klaukecd .git/hooks
7096ea6c27SSimon Basléwget http://review.couchbase.com/tools/hooks/commit-msg
7196ea6c27SSimon Basléchmod +x commit-msg
7296ea6c27SSimon Baslé```
7396ea6c27SSimon Baslé
7496ea6c27SSimon BasléKeep in mind that in Gerrit (unlike Github's Pull Requests), all iterations of a particular change **must** take place in a single commit.
7596ea6c27SSimon BasléThis is done by using `git commit --amend` (or rebasing and squashing) each time you make alterations to your change.
7696ea6c27SSimon BasléThe discussion takes place inside of Gerrit's UI (it will provide you the URL to the change when pushing), and the history
7796ea6c27SSimon Basléof the change is internally maintained by Gerrit so you can actually compare each revision despite having used --amend.
7896ea6c27SSimon Baslé
7996ea6c27SSimon Baslé## Making the Change
8096ea6c27SSimon BasléThe previous step is a one-time configuration of the repository. The following must be done for each new contribution.
8196ea6c27SSimon Baslé
8296ea6c27SSimon BasléBefore you start coding, you should usually open an issue in the [Couchbase bug tracker](https://issues.couchbase.com/projects/JCBC/)
8396ea6c27SSimon Baslé(JIRA), under the relevant project: `JCBC` for Couchbase Java Client (java-client) and `JVMCBC` for Java Couchbase JVM
8496ea6c27SSimon BasléCore (core-io). That may avoid unnecessary effort, for example if the change you are planning to make is going to be
8596ea6c27SSimon Basléobsolete because of a modification we've already planned.
8696ea6c27SSimon Baslé
8796ea6c27SSimon BasléThis will also give the change an issue number that you can reference in the commit.
8896ea6c27SSimon Baslé
8996ea6c27SSimon BasléTo prepare a patch for the `master` branch, start from it and preferably create a new branch for your patch:
9096ea6c27SSimon Baslé
9196ea6c27SSimon Baslé```bash
9296ea6c27SSimon Baslé# this will be kept local, so you can use a very simple branch name
9396ea6c27SSimon Baslégit checkout master -b myPatch
9496ea6c27SSimon Baslé# or a fancier branch name :-)
9596ea6c27SSimon Baslégit checkout master -b contribs/JCBC-XXX-myPatch
9696ea6c27SSimon Baslé```
9796ea6c27SSimon Baslé
9896ea6c27SSimon BasléMake your changes and do the first commit, it will be issued a Change-Id:
9996ea6c27SSimon Baslé
10096ea6c27SSimon Baslé```
10196ea6c27SSimon Baslégit commit
10296ea6c27SSimon Baslé```
10396ea6c27SSimon Baslé
10496ea6c27SSimon BasléYour preferred text editor should open for you to fill in the commit message. Note that we use a template for commit
10596ea6c27SSimon Baslémessages that looks like this:
10696ea6c27SSimon Baslé
10796ea6c27SSimon Baslé```txt
10896ea6c27SSimon Baslé##IDEAL SIZE FOR COMMIT TITLE (50char)###########
10996ea6c27SSimon Baslé#<TICKET-NR>: Commit Short Info, goes in Changelog
11096ea6c27SSimon Baslé
11196ea6c27SSimon Baslé#Motivation
11296ea6c27SSimon Baslé#----------
11396ea6c27SSimon Baslé## UNCOMMENT THE ABOVE IF THE SECTION IS RELEVANT
11496ea6c27SSimon Baslé
11596ea6c27SSimon Baslé##IDEAL SIZE FOR BODY (72char)#########################################
11696ea6c27SSimon Baslé# A few lines about why this change is needed at all. Probably a bug
11796ea6c27SSimon Baslé# that has shown up or why the enhancement, or new feature makes sense.
11896ea6c27SSimon Baslé
11996ea6c27SSimon Baslé#Modifications
12096ea6c27SSimon Baslé#-------------
12196ea6c27SSimon Baslé## UNCOMMENT THE ABOVE IF THE SECTION IS RELEVANT
12296ea6c27SSimon Baslé
12396ea6c27SSimon Baslé##IDEAL SIZE FOR BODY (72char)#########################################
12496ea6c27SSimon Baslé# Explicit info about what has changed, more comments on the code that
12596ea6c27SSimon Baslé# has changed and explicitly state breaking changes or things that
12696ea6c27SSimon Baslé# impact users.
12796ea6c27SSimon Baslé
12896ea6c27SSimon Baslé#Result
12996ea6c27SSimon Baslé#------
13096ea6c27SSimon Baslé## UNCOMMENT THE ABOVE IF THE SECTION IS RELEVANT
13196ea6c27SSimon Baslé
13296ea6c27SSimon Baslé##IDEAL SIZE FOR BODY (72char)#########################################
13396ea6c27SSimon Baslé# What’s the outcome of the change? For example if there is a
13496ea6c27SSimon Baslé# performance optimization adding before/after numbers here make sense.
13596ea6c27SSimon Baslé```
13696ea6c27SSimon Baslé
13796ea6c27SSimon Baslé>**`TICKET-NR`** is the Jira issue id you have created, eg. `JCBC-123`.
13896ea6c27SSimon Baslé>
13996ea6c27SSimon Baslé> If you want to verify that the Change-Id was correctly generated, look at the end of the commit message in `git log`,
14096ea6c27SSimon Baslé> it should appear there.
14196ea6c27SSimon Baslé
14296ea6c27SSimon BasléExample of a very short commit message that follows this template:
14396ea6c27SSimon Baslé
14496ea6c27SSimon Baslé```txt
14596ea6c27SSimon BasléJCBC-123: Fix MadeUpClass array out of bounds
14696ea6c27SSimon Baslé
14796ea6c27SSimon BasléMotivation
14896ea6c27SSimon Baslé----------
14996ea6c27SSimon BasléThe MadeUpClass uses an array of 128 ints internally, but sometimes we
15096ea6c27SSimon Basléquery it with an index of 256...
15196ea6c27SSimon Baslé
15296ea6c27SSimon BasléModifications
15396ea6c27SSimon Baslé-------------
15496ea6c27SSimon BasléUse Math.min to limit the index we query with.
15596ea6c27SSimon Baslé
15696ea6c27SSimon BasléAdded a unit test to avoid regressions on this bug in the future.
15796ea6c27SSimon Baslé
15896ea6c27SSimon BasléResult
15996ea6c27SSimon Baslé------
16096ea6c27SSimon BasléNo more ArrayIndexOutOfBoundsException. This bug is now tested against.
16196ea6c27SSimon Baslé```
16296ea6c27SSimon Baslé
16396ea6c27SSimon Baslé> Notice the IDEAL SIZE lines in the template have the recommended maximum length for the section.
16496ea6c27SSimon Baslé>
16596ea6c27SSimon Baslé> Notice as well that in the above we uncommented the section headers and filled something in for each section.
16696ea6c27SSimon Baslé
16796ea6c27SSimon BasléIf you want to add modifications or you come back to the change later (eg. because of discussions in Gerrit), **don't
16896ea6c27SSimon Basléforget to use `--amend`** (and don't modify the last lines where Gerrit metadata are appended, especially the Change-Id):
16996ea6c27SSimon Baslé
17096ea6c27SSimon Baslé```bash
17196ea6c27SSimon Baslé# to also edit the commit message:
17296ea6c27SSimon Baslégit commit --amend
17396ea6c27SSimon Baslé# if the commit message is already good:
17496ea6c27SSimon Baslégit commit --amend --no-edit
17596ea6c27SSimon Baslé```
17696ea6c27SSimon Baslé
17796ea6c27SSimon BasléFinally, to upload the change to Gerrit (as a new Changeset) or a later modification (as a new Patchset inside the
17896ea6c27SSimon BasléChangeset), push to the Gerrit remote's special branch:
17996ea6c27SSimon Baslé
18096ea6c27SSimon Baslé```bash
18196ea6c27SSimon Baslégit push gerrit HEAD:refs/for/master
18296ea6c27SSimon Baslé```
18396ea6c27SSimon Baslé
18496ea6c27SSimon Baslé> **Note:** As stated earlier there is a CLI tool, [`git-review`](https://github.com/openstack-infra/git-review)
18596ea6c27SSimon Baslé> from OpenStack for dealing with Gerrit-specific commands. The above would be replaced by `git review -R`.
18696ea6c27SSimon Baslé>
18796ea6c27SSimon Baslé> And it generates the Change-Id, so no need to set up the commit hook manually.
18896ea6c27SSimon Baslé
18923f142b3SFelix KlaukeGerrit should answer with the URL to your Changeset, where you can call for reviewers (for the Java SDK, `Michael Nitschinger`).
19096ea6c27SSimon Baslé
19196ea6c27SSimon BasléTo mark a changeset as ready for review (you are confident the change is complete with code and tests, and you have
19296ea6c27SSimon Basléexecuted all unit tests and preferably all integration tests), you can use the "Reply..." button, top right and give
19396ea6c27SSimon Baslé"`Verified`" a note of +1.
19496ea6c27SSimon Baslé
19596ea6c27SSimon BasléReviewers will be notified and will look at your change, replying with a "`Code-Review`" mark between -2 and +2.
19696ea6c27SSimon BasléIf <= 0, there should be comments visible in the bottom list, starting a discussion to improve the change until a later
19796ea6c27SSimon Baslépatchset can receive a `Code-Review +2` and be merged in.
19896ea6c27SSimon Baslé
19996ea6c27SSimon Baslé## Troubleshooting
20096ea6c27SSimon Baslé### "Upload denied for project"
20196ea6c27SSimon BasléIf you get the following error while attempting to push your first change to Gerrit:
20296ea6c27SSimon Baslé```
20396ea6c27SSimon Basléfatal: Upload denied for project 'couchbase-java-client'
20496ea6c27SSimon Basléfatal: Could not read from remote repository.
20596ea6c27SSimon BasléPlease make sure you have the correct access rights
20696ea6c27SSimon Basléand the repository exists.
20796ea6c27SSimon Baslé```
20896ea6c27SSimon Baslé
20996ea6c27SSimon BasléMake sure that you have accepted the CLA as described in step 3. of the intro. Also please check your ssh configuration.
21096ea6c27SSimon Baslé
21196ea6c27SSimon BasléThis applies both when using bare git (`git push gerrit HEAD:refs/for/master`) or git-review ( `git review -R`).
21239b41060SMichael Nitschinger
21396ea6c27SSimon Baslé## Final Note
21496ea6c27SSimon BasléFinally, feel free to reach out to the maintainers over the forums, IRC or email ([sdk_dev@couchbase.com](mailto:sdk_dev@couchbase.com))
21596ea6c27SSimon Basléif you have further questions on contributing or get stuck along the way. **We love contributions and want to help you
21669436efbSSergey Avseyevget your change over the finish line - and you mentioned in the release notes!**
217