From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp55.i.mail.ru (smtp55.i.mail.ru [217.69.128.35]) (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 1C6BC42F4AD for ; Thu, 18 Jun 2020 12:33:39 +0300 (MSK) Date: Thu, 18 Jun 2020 12:32:14 +0300 From: Sergey Bronnikov Message-ID: <20200618093214.GB89732@pony.bronevichok.ru> References: <1d6128a1-127e-8f27-b937-c88219b3f53f@tarantool.org> <20200611120049.GA66555@pony.bronevichok.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 5/6] Fix luacheck warnings in src/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, 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@