Merge branch 'master' into issue-board-sidebarissue-board-sidebar

5 files changed, 139 insertions, 8 deletions

diff --git a/doc/development/README.md b/doc/development/README.md
index 9706cb1de7f..14d6f08e43a 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -8,13 +8,16 @@
 
 ## Styleguides
 
+- [API styleguide](api_styleguide.md) Use this styleguide if you are
+  contributing to the API.
 - [Documentation styleguide](doc_styleguide.md) Use this styleguide if you are
   contributing to documentation.
 - [SQL Migration Style Guide](migration_style_guide.md) for creating safe SQL migrations
 - [Testing standards and style guidelines](testing.md)
 - [UI guide](ui_guide.md) for building GitLab with existing CSS styles and elements
 - [Frontend guidelines](frontend.md)
-- [SQL guidelines](sql.md) for SQL guidelines
+- [SQL guidelines](sql.md) for working with SQL queries
+- [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers
 
 ## Process
 
diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md
new file mode 100644
index 00000000000..ce444ebdde4
--- /dev/null
+++ b/doc/development/api_styleguide.md
@@ -0,0 +1,96 @@
+# API styleguide
+
+This styleguide recommends best practices for API development.
+
+## Instance variables
+
+Please do not use instance variables, there is no need for them (we don't need
+to access them as we do in Rails views), local variables are fine.
+
+## Entities
+
+Always use an [Entity] to present the endpoint's payload.
+
+## Methods and parameters description
+
+Every method must be described using the [Grape DSL](https://github.com/ruby-grape/grape#describing-methods)
+(see https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/api/environments.rb
+for a good example):
+
+- `desc` for the method summary. You should pass it a block for additional
+  details such as:
+  - The GitLab version when the endpoint was added
+  - If the endpoint is deprecated, and if so, when will it be removed
+
+- `params` for the method params. This acts as description,
+  [validation, and coercion of the parameters]
+
+A good example is as follows:
+
+```ruby
+desc 'Get all broadcast messages' do
+  detail 'This feature was introduced in GitLab 8.12.'
+  success Entities::BroadcastMessage
+end
+params do
+  optional :page,     type: Integer, desc: 'Current page number'
+  optional :per_page, type: Integer, desc: 'Number of messages per page'
+end
+get do
+  messages = BroadcastMessage.all
+
+  present paginate(messages), with: Entities::BroadcastMessage
+end
+```
+
+## Declared params
+
+> Grape allows you to access only the parameters that have been declared by your
+`params` block. It filters out the params that have been passed, but are not
+allowed.
+
+– https://github.com/ruby-grape/grape#declared
+
+### Exclude params from parent namespaces!
+
+> By default `declared(params) `includes parameters that were defined in all
+parent namespaces.
+
+– https://github.com/ruby-grape/grape#include-parent-namespaces
+
+In most cases you will want to exclude params from the parent namespaces:
+
+```ruby
+declared(params, include_parent_namespaces: false)
+```
+
+### When to use `declared(params)`?
+
+You should always use `declared(params)` when you pass the params hash as
+arguments to a method call.
+
+For instance:
+
+```ruby
+# bad
+User.create(params) # imagine the user submitted `admin=1`... :)
+
+# good
+User.create(declared(params, include_parent_namespaces: false).to_h)
+```
+
+>**Note:**
+`declared(params)` return a `Hashie::Mash` object, on which you will have to
+call `.to_h`.
+
+But we can use `params[key]` directly when we access single elements.
+
+For instance:
+
+```ruby
+# good
+Model.create(foo: params[:foo])
+```
+
+[Entity]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/api/entities.rb
+[validation, and coercion of the parameters]: https://github.com/ruby-grape/grape#parameter-validation-and-coercion
diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md
index f07d2c9af2d..4cc581dd991 100644
--- a/doc/development/doc_styleguide.md
+++ b/doc/development/doc_styleguide.md
@@ -342,12 +342,6 @@ You can use the following fake tokens as examples.
 Here is a list of must-have items. Use them in the exact order that appears
 on this document. Further explanation is given below.
 
-- Every method must be described using [Grape's DSL](https://github.com/ruby-grape/grape/tree/v0.13.0#describing-methods)
-  (see https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/api/environments.rb
-  for a good example):
-  - `desc` for the method summary (you can pass it a block for additional details)
-  - `params` for the method params (this acts as description **and** validation
-    of the params)
 - Every method must have the REST API request. For example:
 
     ```
diff --git a/doc/development/performance.md b/doc/development/performance.md
index c4a964d1da3..8337c2d9cb3 100644
--- a/doc/development/performance.md
+++ b/doc/development/performance.md
@@ -37,7 +37,7 @@ graphs/dashboards.
 GitLab provides built-in tools to aid the process of improving performance:
 
 * [Sherlock](profiling.md#sherlock)
-* [GitLab Performance Monitoring](../monitoring/performance/monitoring.md)
+* [GitLab Performance Monitoring](../administration/monitoring/performance/monitoring.md)
 * [Request Profiling](../administration/monitoring/performance/request_profiling.md)
 
 GitLab employees can use GitLab.com's performance monitoring systems located at
diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md
new file mode 100644
index 00000000000..e3a20f29a09
--- /dev/null
+++ b/doc/development/sidekiq_style_guide.md
@@ -0,0 +1,38 @@
+# Sidekiq Style Guide
+
+This document outlines various guidelines that should be followed when adding or
+modifying Sidekiq workers.
+
+## Default Queue
+
+Use of the "default" queue is not allowed. Every worker should use a queue that
+matches the worker's purpose the closest. For example, workers that are to be
+executed periodically should use the "cronjob" queue.
+
+A list of all available queues can be found in `config/sidekiq_queues.yml`.
+
+## Dedicated Queues
+
+Most workers should use their own queue. To ease this process a worker can
+include the `DedicatedSidekiqQueue` concern as follows:
+
+```ruby
+class ProcessSomethingWorker
+  include Sidekiq::Worker
+  include DedicatedSidekiqQueue
+end
+```
+
+This will set the queue name based on the class' name, minus the `Worker`
+suffix. In the above example this would lead to the queue being
+`process_something`.
+
+In some cases multiple workers do use the same queue. For example, the various
+workers for updating CI pipelines all use the `pipeline` queue. Adding workers
+to existing queues should be done with care, as adding more workers can lead to
+slow jobs blocking work (even for different jobs) on the shared queue.
+
+## Tests
+
+Each Sidekiq worker must be tested using RSpec, just like any other class. These
+tests should be placed in `spec/workers`.