From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [94.100.177.89]) (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 D12694696C3 for ; Wed, 15 Apr 2020 11:30:03 +0300 (MSK) Date: Wed, 15 Apr 2020 11:30:02 +0300 From: Sergey Bronnikov Message-ID: <20200415083002.GB40826@pony.bronevichok.ru> References: <03c926eb-4582-e651-9524-eec5815c4ee7@tarantool.org> <20200413151601.GA89111@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 v2 5/6] Add luacheck config List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: o.piskunov@tarantool.org, Sergey Bronnikov , tarantool-patches@dev.tarantool.org Hi, Vladislav, see comments inline. On 01:29 Wed 15 Apr , Vladislav Shpilevoy wrote: > 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. Got it. > But I will leave it to a second reviewer. Igor wanted to take a look too. Well, I'll wait Igor and then update patchset accordingly. S.