From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp43.i.mail.ru (smtp43.i.mail.ru [94.100.177.103]) (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 9866A42F4AD for ; Thu, 11 Jun 2020 15:02:18 +0300 (MSK) Date: Thu, 11 Jun 2020 15:01:03 +0300 From: Sergey Bronnikov Message-ID: <20200611120103.GA83685@pony.bronevichok.ru> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 6/6] Fix luacheck warnings in src/box/lua/ List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: alexander.turenko@tarantool.org, tarantool-patches@dev.tarantool.org Hi, thanks for review! On 20:03 Sat 06 Jun , Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 7 comments below. > > On 04/06/2020 10:39, sergeyb@tarantool.org wrote: > > From: Sergey Bronnikov > > > > Part of #4681 > > > > Reviewed-by: Vladislav Shpilevoy > > Co-authored-by: Vladislav Shpilevoy > > --- > > .luacheckrc | 10 +++++-- > > src/box/lua/console.lua | 10 +++---- > > src/box/lua/feedback_daemon.lua | 2 +- > > src/box/lua/key_def.lua | 2 +- > > src/box/lua/load_cfg.lua | 11 ++++--- > > src/box/lua/net_box.lua | 52 +++++++++++++-------------------- > > src/box/lua/schema.lua | 44 +++++++++++++--------------- > > src/box/lua/tuple.lua | 8 ++--- > > src/box/lua/upgrade.lua | 19 ++++++------ > > 9 files changed, 73 insertions(+), 85 deletions(-) > > > > diff --git a/.luacheckrc b/.luacheckrc > > index fb8b9dfb3..9fd017629 100644 > > --- a/.luacheckrc > > +++ b/.luacheckrc > > @@ -19,7 +20,7 @@ include_files = { > > > > exclude_files = { > > "build/**/*.lua", > > - "src/box/**/*.lua", > > + "src/box/lua/serpent.lua", -- third-party source code > > 1. I see the comments are getting less and less strict in every new patch. > Please, be consistent. Use a capital letter to start a sentence, and end it > with dot. Fixed. > > "test-run/**/*.lua", > > "test/**/*.lua", > > "third_party/**/*.lua", > > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua > > index 9560bfdd4..f71744014 100644 > > --- a/src/box/lua/net_box.lua > > +++ b/src/box/lua/net_box.lua > > @@ -187,7 +183,7 @@ local function next_id(id) return band(id + 1, 0x7FFFFFFF) end > > -- @retval two non-nils A connected socket and a decoded greeting. > > -- > > local function establish_connection(host, port, timeout) > > - local timeout = timeout or DEFAULT_CONNECT_TIMEOUT > > + timeout = timeout or DEFAULT_CONNECT_TIMEOUT > > 2. Unnecessary diff. I reverted this line and nothing changed. change is required, see [1] src/box/lua/net_box.lua:186:11: variable timeout was previously defined as an argument on line 185 1. https://gitlab.com/tarantool/tarantool/-/jobs/589857153 > > local begin = fiber.clock() > > local s = socket.tcp_connect(host, port, timeout) > > if not s then > > diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua > > index cdfbb65f7..4fe0ff8da 100644 > > --- a/src/box/lua/schema.lua > > +++ b/src/box/lua/schema.lua > > @@ -846,7 +844,7 @@ local function space_sequence_alter_prepare(format, parts, options, > > if id == nil then > > box.error(box.error.NO_SUCH_SEQUENCE, new_sequence.id) > > end > > - local tuple = _space_sequence.index.sequence:select(id)[1] > > + tuple = _space_sequence.index.sequence:select(id)[1] > > 3. Ditto. change is reverted > > if tuple ~= nil and tuple.is_generated then > > box.error(box.error.ALTER_SPACE, space_name, > > "can not attach generated sequence") > > @@ -2681,7 +2679,7 @@ box.once = function(key, func, ...) > > box.error(box.error.ILLEGAL_PARAMS, "Usage: box.once(key, func, ...)") > > end > > > > - local key = "once"..key > > + key = "once"..key > > 4. Ditto. change is reverted > > if box.space._schema:get{key} ~= nil then > > return > > end > > diff --git a/src/box/lua/tuple.lua b/src/box/lua/tuple.lua > > index f97aa1579..87f53258b 100644 > > --- a/src/box/lua/tuple.lua > > +++ b/src/box/lua/tuple.lua > > @@ -232,7 +230,7 @@ local function tuple_update(tuple, expr) > > error("Usage: tuple:update({ { op, field, arg}+ })") > > end > > local pexpr, pexpr_end = tuple_encode(expr) > > - local tuple = builtin.box_tuple_update(tuple, pexpr, pexpr_end) > > + tuple = builtin.box_tuple_update(tuple, pexpr, pexpr_end) > > 5. Ditto. change is required, see [1] 1. https://gitlab.com/tarantool/tarantool/-/jobs/589857153 > > > if tuple == nil then > > return box.error() > > end > > @@ -245,7 +243,7 @@ local function tuple_upsert(tuple, expr) > > error("Usage: tuple:upsert({ { op, field, arg}+ })") > > end > > local pexpr, pexpr_end = tuple_encode(expr) > > - local tuple = builtin.box_tuple_upsert(tuple, pexpr, pexpr_end) > > + tuple = builtin.box_tuple_upsert(tuple, pexpr, pexpr_end) > > 6. Ditto. change is required, see [1] 1. https://gitlab.com/tarantool/tarantool/-/jobs/589857153 > > if tuple == nil then > > return box.error() > > end > > diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua > > index 075cc236e..208b53f15 100644 > > --- a/src/box/lua/upgrade.lua > > +++ b/src/box/lua/upgrade.lua > > @@ -580,7 +579,7 @@ local function upgrade_to_2_1_0() > > -- Now, abscent field means NULL, so we can safely set second > > -- field in format, marking it nullable. > > log.info("Add nullable value field to space _schema") > > - local format = {} > > + format = {} > > 7. Ditto. change is reverted > > format[1] = {type='string', name='key'} > > format[2] = {type='any', name='value', is_nullable=true} > > box.space._schema:format(format)