Add contributing guideline document
Change-Id: Ic4a7c07de198bbbaf12e9d231a65a51f3fceae52
diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt
new file mode 100644
index 0000000..fd98c1d
--- /dev/null
+++ b/Documentation/dev-contributing.txt
@@ -0,0 +1,241 @@
+Gerrit Code Review - Contributing
+=================================
+
+Gerrit is developed as a self-hosting open source project and
+very much welcomes contributions from anyone with a contributor's
+agreement on file with the project.
+
+* https://gerrit-review.googlesource.com/
+
+The Contributor License Agreements:
+
+* https://gerrit-review.googlesource.com/static/cla_individual.html
+* https://gerrit-review.googlesource.com/static/cla_corporate.html
+
+As Gerrit is a code review tool, naturally contributions will
+be reviewed before they will get submitted to the code base. To
+start your contribution, please make a git commit and upload it
+for review to the main Gerrit review server. To help speed up the
+review of your change, review these guidelines before submitting
+your change. You can view the pending Gerrit contributions and
+their statuses here:
+
+* https://gerrit-review.googlesource.com/#/q/status:open+project:gerrit,n,z
+
+Depending on the size of that list it might take a while for
+your change to get reviewed. Naturally there are fewer
+approvers than contributors; so anything that you can do to
+ensure that your contribution will undergo fewer revisions
+will speed up the contribution process. This includes helping
+out reviewing other people's changes to relieve the load from
+the approvers. Even if you are not familiar with Gerrit's
+internals, it would be of great help if you can download, try
+out, and comment on new features. If it works as advertised,
+say so, and if you have the priviliges to do so, go ahead
+and give it a +1 Verified. If you would find the feature
+useful, say so and give it a +1 code review.
+
+And finally, the quicker you respond to the comments of your
+reviewers, the quicker your change might get merged! Try to
+reply to every comment after submitting your new patch,
+particularly if you decided against making the suggested change.
+Reviewers don't want to seem like nags and pester you if you
+haven't replied or made a fix, so it helps them know if you
+missed it or decided against it.
+
+
+Review Criteria
+---------------
+
+Here are some hints as to what approvers may be looking for
+before approving or submitting changes to the Gerrit project.
+Let's start with the simple nit picky stuff. You are likely
+excited that your code works; help us share your excitement
+by not distracting us with the simple stuff. Thanks to Gerrit,
+problems are often highlighted and we find it hard to look
+beyond simple spacing issues. Blame it on our short attention
+spans, we really do want your code.
+
+
+Commit Message
+--------------
+
+It is essential to have a good commit message if you want your
+change to be reviewed.
+
+ * Keep lines no longer than 72 chars
+ * Start with a short one line summary
+ * Followed by a blank line
+ * Followed by one or more explanatory paragraphs
+ * Use the present tense (fix instead of fixed)
+ * Include a Bug: Issue <#> line if fixing a Gerrit issue
+ * Include a Change-Id line
+
+
+A sample good Gerrit commit message:
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+====
+ Add sample commit message to guidelines doc
+
+ The original patch set for the contributing guidelines doc did not
+ include a sample commit message, this new patchset does. Hopefully this
+ makes things a bit clearer since examples can sometimes help when
+ explanations don't.
+
+ Note that the body of this commit message can be several paragraphs, and
+ that I word wrap it at 72 characters. Also note that I keep the summary
+ line under 50 characters since it is often truncated by tools which
+ display just the git summary.
+
+ Bug: Issue 98765605
+ Change-Id: Ic4a7c07eeb98cdeaf44e9d231a65a51f3fceae52
+====
+
+
+Style
+-----
+
+The basic coding style is covered by the tools/GoogleFormat.xml
+doc, see the link:dev-eclipse.html#Formatting[Eclipse Setup]
+for that.
+
+Highlighted/additional styling notes:
+
+ * It is generally more important to match the style of the nearby
+ code which you are modifying than it is to match the style
+ in the formatting guidelines. This is especially true within the
+ same file.
+ * Review your change in Gerrit to see if it highlights
+ mistakingly deleted/added spaces on lines, trailing spaces.
+ * Line length should be 80 or less, unless the code reads
+ better with something slightly longer. Shorter lines not only
+ help reviewers who may use a tablet to review the code, but future
+ contributors may also like to open several editors side by
+ side while editing new changes.
+ * Use 2 spaces for indent (no tabs)
+ * Use brackets in all ifs, spaces before/after if parens.
+ * Use /** */ style Javadocs for variables.
+
+Additionally, you will notice that most of the newline spacing
+is fairly consistent throughout the code in Gerrit, it helps to
+stick to the blank line conventions. Here are some specific
+examples:
+
+ * Keep a blank line between all class and method declarations.
+ * Do not add blank lines at the beginning or end of class/methods.
+ * Put a blank line between external import sources, but not
+ between internal ones.
+
+
+Code Organization
+-----------------
+
+Do your best to organize classes and methods in a logical way.
+Here are some guidelines that Gerrit uses:
+
+ * Ensure a standard copyright header is included at the top
+ of any new files (copy it from another file, update the year).
+ * Always place loggers first in your class!
+ * Define any static interfaces next in your class.
+ * Define non static interfaces after static interfaces in your
+ class.
+ * Next you should define static types and members.
+ * Finally instance members, then constuctors, and then instance
+ methods.
+ * Some common exceptions are private helper static methods which
+ might appear near the instance methods which they help.
+ * Getters and setters for the same instance field should usually
+ be near each other baring a good reason not to.
+ * If you are using assisted injection, the factory for your class
+ should be before the instance members.
+ * Imports should be mostly aphabetical (uppercase sorts before
+ all lowercase, which means classes come before packages at the
+ same level).
+
+Wow that's a lot! But don't worry, you'll get the habit and most
+of the code is organized this way already; so if you pay attention
+to the class you are editing you will likely pick up on it.
+Naturally new classes are a little harder; you may want to come
+back and consult this section when creating them.
+
+
+Design
+------
+
+Here are some design level ojectives that you should keep in mind
+when coding:
+
+ * ORM entity objects should match exactly one row in the database.
+ * Most client pages should perform only one RPC to load so as to
+ keep latencies down. Exceptions would apply to RPCs which need
+ to load large data sets if splitting them out will help the
+ page load faster. Generally page loads are expected to complete
+ in under 100ms. This will be the case for most operations,
+ unless the data being fetched is not using Gerrit's caching
+ infrastructure. In these slower cases, it is worth considering
+ mitigating this longer load by using a second RPC to fill in
+ this data after the page is displayed (or alternatively it might
+ be worth proposing caching this data).
+ * @Inject should be used on constructors, not on fields. The
+ current exceptions are the ssh commands, these were implemented
+ earlier in Gerrit's development. To stay consistent, new ssh
+ commands should follow this older pattern; but eventually these
+ should get converted to eliminate this exception.
+ * Don't leave repository objects (git or schema) open. A .close()
+ after every open should be placed in a finally{} block.
+ * Don't leave UI components, which can cause new actions to occur,
+ enabled during RPCs which update the DB. This is to prevent
+ people from submitting actions more than once when operating
+ on slow links. If the action buttons are disabled, they cannot
+ be resubmitted and the user can see that Gerrit is still busy.
+ * GWT EventBus is the new way forward.
+
+
+Tests
+-----
+
+ * Tests for new code will greatly help your change get approved.
+
+
+Change Size/Number of Files Touched
+-----------------------------------
+
+And finally, I probably cannot say enough about change sizes.
+Generally, smaller is better, hopefully within reason. Do try to
+keep things which will be confusing on their own together,
+especially if changing one without the other will break something!
+
+ * If a new feature is implemented and it is a larger one, try to
+ identify if it can be split into smaller logical features; when
+ in doubt, err on the smaller side.
+ * Separate bug fixes from feature improvements. The bug fix may
+ be an easy candidate for approval and should not need to wait
+ for new features to be approved. Also, combining the two makes
+ reviewing harder since then there is no clear line between the
+ fix and the feature.
+ * Separate supporting refactoring from feature changes. If your
+ new feature requires some refactoring, it helps to make the
+ refactoring a separate change which your feature change
+ depends on. This way, reviewers can easily review the refactor
+ change as a something that should not alter the current
+ functionality, and feel more confident they can more easily
+ spot errors this way. Of course, it also makes it easier to
+ test and locate later on if an unfortunate error does slip in.
+ Lastly, by not having to see refactoring changes at the same
+ time, it helps reviewers understand how your feature changes
+ the current functionality.
+ * Separate logical features into separate changes. This
+ is often the hardest part. Here is an example: when adding a
+ new ability, make separate changes for the UI and the ssh
+ commands if possible.
+ * Do only what the commit message describes. In other words, things which
+ are not strictly related to the commit message shouldn't be part of
+ a change, even trivial things like externalizing a string somewhere
+ or fixing a typo. This help keep "git blame" more useful in the future
+ and it also makes "git revert" more useful.
+ * Use topic branches to link your separate changes together.
+
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/dev-eclipse.txt b/Documentation/dev-eclipse.txt
index 82d0a67..b37223c 100644
--- a/Documentation/dev-eclipse.txt
+++ b/Documentation/dev-eclipse.txt
@@ -16,6 +16,7 @@
http://m2eclipse.codehaus.org/[m2eclipse]
+[[Formatting]]
Code Formatter Settings
-----------------------
diff --git a/Documentation/index.txt b/Documentation/index.txt
index b86d4b3..5d3eb4c 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -45,6 +45,7 @@
* link:dev-readme.html[Developer Setup]
* link:dev-eclipse.html[Eclipse Setup]
+* link:dev-contributing.html[Contributing to Gerrit]
* link:dev-design.html[System Design]
* link:i18n-readme.html[i18n Support]
