Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org, roman.habibov1@yandex.ru
Subject: [tarantool-patches] Re: [PATCH v3] json: add options to json.encode()
Date: Wed, 8 Aug 2018 22:07:43 +0300	[thread overview]
Message-ID: <f6ca7860-dd90-f747-c33a-38717f41f5ec@tarantool.org> (raw)
In-Reply-To: <26831531533678724@myt5-f9d71769b752.qloud-c.yandex.net>

Hi! Thanks for the fixes!

See below last (I hope) 3 comments.

> commit f6d41bdfcec241733f98b7d26715ece6625539aa
> Author: Roman Khabibov <roman.habibov1@yandex.ru>
> Date:   Sun Jul 8 02:21:08 2018 +0300
> 
>      json: add options to json.encode()
>      
>      Add an ability to pass options to json.encode()/decode().
>      
>      Closes: #2888.

1. Put here a documentation bot request. We have a bot that tracks
all new commits and when in a one it sees a documentation request,
and the commit is pushed into the master, the bot opens an issue
in tarantool/doc repository. So the documentation writers team is
able to reflect your changes on site tarantool.io.

Your patch has changed public API and requires such documentation
request. For syntax see this:
https://github.com/tarantool/docbot#docbot---tarantool-documentation-pipeline-bot

For example of a request see this:
https://www.freelists.org/post/tarantool-patches/PATCH-78-box-introduce-boxctlpromote
where "@TarantoolBot document" is written.

For example of a result see this:
https://github.com/tarantool/doc/issues/created_by/TarantoolBot

Here you can check was your request successful or not (by syntax, for
instance): http://try.tarantool.org:11116

> 
> diff --git a/src/lua/utils.c b/src/lua/utils.c
> index 2f0f4dcf8..3faeb416f 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -214,6 +214,48 @@ static struct {
>   	{ NULL, 0, 0, 0},
>   };
>   
> +/**
> + * Configure one field in @a cfg.
> + * @param L lua stack
> + * @param i index of option in OPTIONS[]
> + * @param cfg serializer to inherit configuration
> + * @retval pointer to the value of option
> + * @retval NULL if option is not> + * in the table

2. As I've already said, please, start a sentence from a capital letter
and finish with the dot. For example see other files:
https://github.com/tarantool/tarantool/blob/2.0/src/box/tuple.h#L367

In other places too.

> diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua
> index 42c79d6e9..f2c67ab0a 100755
> --- a/test/app-tap/json.test.lua
> +++ b/test/app-tap/json.test.lua
> @@ -22,7 +22,55 @@ end
>   
>   tap.test("json", function(test)
>       local serializer = require('json')
> -    test:plan(13)
> +    test:plan(25)
> +
> +-- gh-2888: Check the possibility of using options in encode()/decode().

3. From the previous review point 5 still is not fixed. Your comment
starts earlier than the function. But should be aligned by the function
body, like usual statement. (Add 4 spaces before the comment.)

  parent reply	other threads:[~2018-08-08 19:07 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1531010828.git.roman.habibov1@yandex.ru>
2018-07-08  0:57 ` [tarantool-patches] [PATCH] json: added " Roman Khabibov
2018-07-09 10:33   ` [tarantool-patches] " Vladislav Shpilevoy
2018-07-17 18:19     ` roman.habibov1
2018-07-19 10:18       ` Vladislav Shpilevoy
2018-07-23 22:38         ` [tarantool-patches] Re: [PATCH v3] json: add " roman.habibov1
2018-07-25 21:35           ` Vladislav Shpilevoy
2018-07-26  9:40             ` roman.habibov1
2018-07-26 10:07               ` Vladislav Shpilevoy
2018-07-26 12:29                 ` roman.habibov1
2018-07-26 12:33                   ` Vladislav Shpilevoy
2018-07-26 13:19                     ` roman.habibov1
2018-07-26 21:45                       ` Vladislav Shpilevoy
2018-07-31 15:29                         ` roman.habibov1
2018-08-01 10:37                           ` Vladislav Shpilevoy
2018-08-01 20:41                             ` roman.habibov1
2018-08-02 12:59                               ` Vladislav Shpilevoy
2018-08-07 21:52                                 ` roman.habibov1
2018-08-07 21:53                                   ` roman.habibov1
2018-08-08 19:07                                   ` Vladislav Shpilevoy [this message]
2018-08-13 23:14                                     ` roman.habibov1
2018-08-14 22:29                                       ` Vladislav Shpilevoy
2018-08-23 21:03                                         ` Alexander Turenko
2018-09-09 15:28                                         ` Alexander Turenko
2018-09-09 23:42                                           ` roman.habibov1
2018-09-10 13:12                                             ` Alexander Turenko
2018-08-08 19:08                                 ` Vladislav Shpilevoy
2018-07-11  7:57   ` [tarantool-patches] Re: [PATCH] json: added " Kirill Yukhin
2018-07-19 10:24   ` Vladislav Shpilevoy
2018-09-13 15:23   ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f6ca7860-dd90-f747-c33a-38717f41f5ec@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=roman.habibov1@yandex.ru \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v3] json: add options to json.encode()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox