Re: [Tarantool-patches] [PATCH v2 5/6] Add luacheck config

[Sergey Bronnikov <sergeyb@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.