From b2c771f45261e1484158d1304cd898e866002f07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 13 Oct 2016 18:44:52 +0200 Subject: 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 --- 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 -- cgit v1.2.1 From c1dd1795ed57c9481a1a9cab81daef39dd218346 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 13 Oct 2016 19:35:57 +0200 Subject: 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 --- 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: ``` -- cgit v1.2.1 From 3a1d9bccd6c3ce8249e90153f4299db1c037eb56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 13 Oct 2016 19:38:38 +0200 Subject: Fix typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- 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 -- cgit v1.2.1 From b4810fd2bce42d66cada56af3ef697219f176b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 13 Oct 2016 19:40:20 +0200 Subject: More improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- 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 -- cgit v1.2.1 From 11e40c0e26e07d153cd2712bf23b5d679b54615c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 17 Oct 2016 10:10:13 +0200 Subject: Improve copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- 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: -- cgit v1.2.1