From b2c771f45261e1484158d1304cd898e866002f07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Thu, 13 Oct 2016 18:44:52 +0200
Subject: [PATCH 1/5] Add an API styleguide
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 doc/development/README.md         |  2 ++
 doc/development/api_styleguide.md | 58 +++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100644 doc/development/api_styleguide.md

diff --git a/doc/development/README.md b/doc/development/README.md
index 9706cb1de7f..630fe64cee6 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -8,6 +8,8 @@
 
 ## 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
diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md
new file mode 100644
index 00000000000..ee5e7ad3988
--- /dev/null
+++ b/doc/development/api_styleguide.md
@@ -0,0 +1,58 @@
+# API styleguide
+
+This styleguide recommends best practices for the API development.
+
+## 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 directly `params[key]` when we access single elements.
+
+For instance:
+
+```ruby
+# good
+Model.create(foo: params[:foo])
+```
+
+>**Note:**
+Since you [should use Grape's DSL to declare params](doc_styleguide.md#method-description), [parameters validation and
+coercion] comes for free!
+
+[parameters validation and coercion]: https://github.com/ruby-grape/grape#parameter-validation-and-coercion
-- 
GitLab


From c1dd1795ed57c9481a1a9cab81daef39dd218346 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Thu, 13 Oct 2016 19:35:57 +0200
Subject: [PATCH 2/5] Move the Grape DSL part from Doc styleguide to API
 styleguide
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Also improve API styleguide

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 doc/development/api_styleguide.md | 48 +++++++++++++++++++++++++++----
 doc/development/doc_styleguide.md |  6 ----
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md
index ee5e7ad3988..be303fc6d39 100644
--- a/doc/development/api_styleguide.md
+++ b/doc/development/api_styleguide.md
@@ -2,6 +2,47 @@
 
 This styleguide recommends best practices for the 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 yes 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
@@ -10,7 +51,7 @@ allowed.
 
 – https://github.com/ruby-grape/grape#declared
 
-### Exclude params from parent namespaces
+### Exclude params from parent namespaces!
 
 > By default `declared(params) `includes parameters that were defined in all
 parent namespaces.
@@ -51,8 +92,5 @@ For instance:
 Model.create(foo: params[:foo])
 ```
 
->**Note:**
-Since you [should use Grape's DSL to declare params](doc_styleguide.md#method-description), [parameters validation and
-coercion] comes for free!
-
+[Entity]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/api/entities.rb
 [parameters validation and coercion]: 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 0b725cf200c..882da2a6562 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:
 
     ```
-- 
GitLab


From 3a1d9bccd6c3ce8249e90153f4299db1c037eb56 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Thu, 13 Oct 2016 19:38:38 +0200
Subject: [PATCH 3/5] Fix typo
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 doc/development/api_styleguide.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md
index be303fc6d39..94047dfe173 100644
--- a/doc/development/api_styleguide.md
+++ b/doc/development/api_styleguide.md
@@ -1,6 +1,6 @@
 # API styleguide
 
-This styleguide recommends best practices for the API development.
+This styleguide recommends best practices for API development.
 
 ## Instance variables
 
-- 
GitLab


From b4810fd2bce42d66cada56af3ef697219f176b87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Thu, 13 Oct 2016 19:40:20 +0200
Subject: [PATCH 4/5] More improvements
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 doc/development/api_styleguide.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md
index 94047dfe173..2a41314d2db 100644
--- a/doc/development/api_styleguide.md
+++ b/doc/development/api_styleguide.md
@@ -22,8 +22,8 @@ for a good example):
   - The GitLab version when the endpoint was added
   - If the endpoint is deprecated, and if yes when will it be removed
 
-- `params` for the method params. This acts as description, validation, and
-  coercion of the parameters
+- `params` for the method params. This acts as description,
+  [validation, and coercion of the parameters]
 
 A good example is as follows:
 
@@ -93,4 +93,4 @@ Model.create(foo: params[:foo])
 ```
 
 [Entity]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/api/entities.rb
-[parameters validation and coercion]: https://github.com/ruby-grape/grape#parameter-validation-and-coercion
+[validation, and coercion of the parameters]: https://github.com/ruby-grape/grape#parameter-validation-and-coercion
-- 
GitLab


From 11e40c0e26e07d153cd2712bf23b5d679b54615c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Mon, 17 Oct 2016 10:10:13 +0200
Subject: [PATCH 5/5] Improve copy
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 doc/development/api_styleguide.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md
index 2a41314d2db..ce444ebdde4 100644
--- a/doc/development/api_styleguide.md
+++ b/doc/development/api_styleguide.md
@@ -20,7 +20,7 @@ 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 yes when will it be removed
+  - 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]
@@ -83,7 +83,7 @@ User.create(declared(params, include_parent_namespaces: false).to_h)
 `declared(params)` return a `Hashie::Mash` object, on which you will have to
 call `.to_h`.
 
-But we can use directly `params[key]` when we access single elements.
+But we can use `params[key]` directly when we access single elements.
 
 For instance:
 
-- 
GitLab