From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 3C27428AA1 for ; Wed, 8 Aug 2018 15:07:49 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tM2yA5pAQxWW for ; Wed, 8 Aug 2018 15:07:49 -0400 (EDT) Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 86A3028A99 for ; Wed, 8 Aug 2018 15:07:48 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3] json: add options to json.encode() References: <8327181531851586@myt4-ec1fcebe7be6.qloud-c.yandex.net> <12c20a86-8a40-4fe6-8dab-7b7b9494a142@tarantool.org> <39430371532385504@iva8-37fc2ad204cd.qloud-c.yandex.net> <3367531532598057@sas1-4b7566131ec9.qloud-c.yandex.net> <0e2c47c2-0276-a1da-c54b-726b64c17aae@tarantool.org> <5209131532608150@myt3-c7e5d17fe013.qloud-c.yandex.net> <2952461532611162@iva5-08e20335b0d9.qloud-c.yandex.net> <33ec689b-9aca-97e9-e56f-c0aee649ad87@tarantool.org> <28658371533050946@myt4-74a8acfc13eb.qloud-c.yandex.net> <898281533156060@myt5-cf6d29327892.qloud-c.yandex.net> <26831531533678724@myt5-f9d71769b752.qloud-c.yandex.net> From: Vladislav Shpilevoy Message-ID: Date: Wed, 8 Aug 2018 22:07:43 +0300 MIME-Version: 1.0 In-Reply-To: <26831531533678724@myt5-f9d71769b752.qloud-c.yandex.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, roman.habibov1@yandex.ru Hi! Thanks for the fixes! See below last (I hope) 3 comments. > commit f6d41bdfcec241733f98b7d26715ece6625539aa > Author: Roman Khabibov > 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.)