Re: [Tarantool-patches] [PATCH 6/6] Fix luacheck warnings in src/box/lua/

[Sergey Bronnikov <sergeyb@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 <sergeyb@tarantool.org>
> > 
> > Part of #4681
> > 
> > Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> > Co-authored-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> > ---
> >  .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)