Add contributing guideline document

Change-Id: Ic4a7c07de198bbbaf12e9d231a65a51f3fceae52

3 files changed, 243 insertions, 0 deletions

diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt
new file mode 100644
index 0000000000..fd98c1dd73
--- /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 82d0a67d9a..b37223c637 100644
--- a/Documentation/dev-eclipse.txt
+++ b/Documentation/dev-eclipse.txt
@@ -16,6 +16,7 @@ Install the Maven Integration plugins:
 http://m2eclipse.codehaus.org/[m2eclipse]
 
 
+[[Formatting]]
 Code Formatter Settings
 -----------------------
 
diff --git a/Documentation/index.txt b/Documentation/index.txt
index b86d4b3a7d..5d3eb4c653 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -45,6 +45,7 @@ Developer Documentation
 
 * 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]