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

[Sergey Bronnikov <sergeyb@tarantool.org>]

Hi,

On 21:52 Thu 11 Jun , Vladislav Shpilevoy wrote:
> Thanks for the fixes!
> 
> >>> +    },
> >>> +}
> >>> diff --git a/src/lua/argparse.lua b/src/lua/argparse.lua
> >>> index faa0ae130..6c8f10fc1 100644
> >>> --- a/src/lua/argparse.lua
> >>> +++ b/src/lua/argparse.lua
> >>> @@ -91,7 +91,8 @@ local function convert_parameter(name, convert_from, convert_to)
> >>>  end
> >>>  
> >>>  local function parameters_parse(t_in, options)
> >>> -    local t_out, t_in = {}, t_in or {}
> >>> +    local t_out = {}
> >>> +    t_in = t_in or {}
> >>
> >> 6. Unnecessary diff. At least when I check on top of the branch.
> > 
> > change is required, see [1]
> > 
> > 1. https://gitlab.com/tarantool/tarantool/-/jobs/589857153
> 
> It reports error 412. It is suppressed globally in .luacheckrc. How
> is it possible? I tried it locally, and the warning is not reported.

> In the job I see that the .luacheck file is outdated - it does not
> contain 412 suppression:
> https://gitlab.com/tarantool/tarantool/-/blob/b435c7d040c7ce160d67afde270c336f1b91fddb/.luacheckrc
> 
> While on the branch in github the warning is suppressed.
> 
> So the diff is unnecessary, after all. Unless the github branch is
> outdated.
> https://github.com/tarantool/tarantool/blob/ligurio/gh-4681-fix-luacheck-warnings-src/.luacheckrc#L15
> 
> The same for all the other places, where you said the change is required.
> In the other commits too.

I'm so sorry, I was wrong and all your suggested diffs have been applied.

> >>>      local stop = fiber.clock() + timeout
> >>>      local dns = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
> >>>          protocol = 'tcp' })
> >>> @@ -1319,7 +1319,7 @@ local function lsocket_tcp_getpeername(self)
> >>>      return peer.host, tostring(peer.port), peer.family:match("AF_(.*)"):lower()
> >>>  end
> >>>  
> >>> -local function lsocket_tcp_settimeout(self, value, mode)
> >>> +local function lsocket_tcp_settimeout(self, value, mode) -- luacheck: no unused args
> >>
> >> 10. Mode is really ignored, and this is not some kind of built-in
> >> function with 'fixed' signature. And it is not used 'virtually',
> >> when there is a variable keeping pointer at one function among
> >> many having the same signature. So what is a purpose of keeping it?
> > 
> > removed mode arg
> 
> Yeah, looks like it is needed, after all. Igor says, it conforms
> to http://w3.impa.br/~diego/software/luasocket/tcp.html#settimeout
> or whatever. Personally, I would remove it anyway. Because in our
> sockets and in our code globally this argument is never needed,
> and does not conform to anything.

Well, kept it as is (with removed arg)

-- 
sergeyb@