From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (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 53D254696C3 for ; Wed, 15 Apr 2020 02:29:11 +0300 (MSK) References: <03c926eb-4582-e651-9524-eec5815c4ee7@tarantool.org> <20200413151601.GA89111@pony.bronevichok.ru> From: Vladislav Shpilevoy Message-ID: Date: Wed, 15 Apr 2020 01:29:07 +0200 MIME-Version: 1.0 In-Reply-To: <20200413151601.GA89111@pony.bronevichok.ru> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 5/6] Add luacheck config List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Bronnikov Cc: o.piskunov@tarantool.org, Sergey Bronnikov , tarantool-patches@dev.tarantool.org Hi! Thanks for the answer! >>> Lua source code in test/ directory contains both errors and warnings and there >>> are a lot of them. Some of these warnings were fixed by previous commits and >>> most part of warnings and errors supressed in luacheck config. Regarding >>> errors: all found errors belongs to a single error class - "Syntax error" >>> (E011). All of them have a single root cause - at the of testscases we write >>> conditions without assignments and it is confuses luacheck. Usually such >>> conditions used together with asserts, but in our tests we uses reference outputs >>> instead of asserts. >> >> 1. So you disabled that warning for tests, right? Then why still many >> statements in the test/ dir are replaced with assertions in one of the >> previous commits? > > My original goal was following: fix dangerous warnings and supress other > of them per file. I followed this strategy and then discovered that E011 > cannot be supressed. In case of errors you should fix them or disable > checking at all for highlighted files/directories. So this is a reason > why some exclusions contains single file and some of them contains file > mask. I decided to keep it as is because more specific supressions are > more convenient to fix when. I still think, that if a whole folder was ignored in exclude_files, then it does not make sense to add individual files from this folder again. Anyway we won't be able to maintain warning ignorance for these files, because they are just invisible for luacheck. But I will leave it to a second reviewer. Igor wanted to take a look too.