Re: [Tarantool-patches] [PATCH v6 00/25] Add static analysis with luacheck

[Alexander Turenko <alexander.turenko@tarantool.org>]

I was asked to summarize my proposals re this patchset.

1. Disable redefinition and shadowing warnings globally (in a config)
   and don't touch 'affected' code. I provided examples why those
   changes are not good.
2. Don't remove unused arguments, where they show a function contract:
   use '-- luacheck: no unused args' or even disable them globally if
   there are many of them.

Personally I use 'luacheck -r' with default config and just visually
filter out warnings re 'box' global and so. Before I use '--no-redefined
--no-unused-args' in a commercial project (24K sloc in Lua now) and in
tarantool/graphql.0 (8K sloc, +15K sloc of tests).

I don't think that reverting or applying of a hundred of hunks is so big
work to discuss whether it worth to don't do this, because amount of
work is more then profit, despite that unneded diffs are discouraged,
some code will became less readable, git blame will show unnecessary
artificial changes, but we'll save a day or even two, but our code
examples would suggest to avoid redefinitions and will push future
developers to produce more such code, but two days or ever three, but
maintenance costs are always bigger then developing costs, but...
blah-blah. C'mon.

Three years ago, when I come into the team, it would not be ever
discussed. If unneded work was made it is would be a signal to think why
it appears and, say, extract doubtful changes into separate commits. Or
keep notes to don't miss parts of related discussions. Or ask possible
reviewers before do large amount of changes. Not 'ask to push it anyway,
because much time was spent'.

It is still my personal opinion: I don't a reviewer here.

WBR, Alexander Turenko.