From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <imun@tarantool.org>
Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
 bits)) (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id D3D2D469710
 for <tarantool-patches@dev.tarantool.org>;
 Thu,  4 Jun 2020 01:51:03 +0300 (MSK)
Date: Thu, 4 Jun 2020 01:41:55 +0300
From: Igor Munkin <imun@tarantool.org>
Message-ID: <20200603224155.GG5745@tarantool.org>
References: <cover.1589321083.git.alexander.turenko@tarantool.org>
 <59dfb8a35bff974f53943342fb08b1a32c71d0bd.1589321083.git.alexander.turenko@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <59dfb8a35bff974f53943342fb08b1a32c71d0bd.1589321083.git.alexander.turenko@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH 3/3] box: always reconfigure box at
	non-first box.cfg()
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

Sasha,

Thanks for the patch, LGTM, except two typos below.

On 13.05.20, Alexander Turenko wrote:
> From: Maria <maria.khaydich@tarantool.org>
> 
> Calling box.cfg{} more than once does not normally cause any errors
> (even though it might not have any effect). In contrast, assigning
> it to some variable and then using it after the box was configured
> caused an error since the method was overwritten by the initial call
> of <load_cfg>.
> 
> The patch fixes this issue making box.cfg behave consistently in both
> scenarios.
> 
> Follow-up #4231
> 
> Co-developed-by: Alexander Turenko <alexander.turenko@tarantool.org>
> ---
>  src/box/lua/load_cfg.lua                      |  8 +++++
>  .../gh-4231-box-cfg-idempotence.test.lua      | 34 +++++++++++++++++++
>  2 files changed, 42 insertions(+)
>  create mode 100755 test/box-tap/gh-4231-box-cfg-idempotence.test.lua
> 

<snipped>

> diff --git a/test/box-tap/gh-4231-box-cfg-idempotence.test.lua b/test/box-tap/gh-4231-box-cfg-idempotence.test.lua
> new file mode 100755
> index 000000000..4f3ba68a6
> --- /dev/null
> +++ b/test/box-tap/gh-4231-box-cfg-idempotence.test.lua
> @@ -0,0 +1,34 @@
> +#!/usr/bin/env tarantool
> +
> +--
> +-- gh-4231: box.cfg is another function (so called <load_cfg>)
> +-- before box is loaded. Usually a user calls box.cfg({<...>}),
> +-- it configures box and replaces box.cfg implementation to one
> +-- that performs box reconfiguration: so further calls to
> +-- box.cfg({<...>}) reconfigures box.

Typo: s/reconfigures/reconfigure/.

> +--
> +-- However it is possible to save box.cfg value (<load_cfg>)
> +-- before box loading and call it after box loading: the behaviour
> +-- should be the same as for box.cfg call: box should be

Typo: There are two colons in one sentence, so I propose simply to
s/:/and/ the last one.

> +-- reconfigured.
> +--
> +

<snipped>

> -- 
> 2.25.0
> 

-- 
Best regards,
IM