From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 9EBC842EF5C for ; Thu, 11 Jun 2020 22:52:04 +0300 (MSK) References: <1d6128a1-127e-8f27-b937-c88219b3f53f@tarantool.org> <20200611120049.GA66555@pony.bronevichok.ru> From: Vladislav Shpilevoy Message-ID: Date: Thu, 11 Jun 2020 21:52:02 +0200 MIME-Version: 1.0 In-Reply-To: <20200611120049.GA66555@pony.bronevichok.ru> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Sergey Bronnikov Cc: alexander.turenko@tarantool.org, tarantool-patches@dev.tarantool.org 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. >>> 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.