Re: [Tarantool-patches] [PATCH 1/2] src: return back import of table.clear() method

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

> src: return back import of table.clear() method

We usually mark a subsystem using a prefix: 'app' or 'lua' would fit
good here.

BTW, why you splitted the code and test changes? I know, there are cons
and pros, but we usually include tests into the same commit as a source
change.

Upside: It works as the documentation of changes; a formal description
of the change in addition to informal one within a commit message.

Downside: A bit more work is required to verify the test on different
tarantool version.

> Import of 'table.clear' module has been removed
> to fix luacheck warning about unused variable in
> commit 3af79e70b5e1e9b1d69b97f3031a299132a02d2f

Nit: We usually use form git_commit_hash ('header of the commit') when
mention a commit.

> and method table.clear() became unavailable in Tarantool.

I would clarify that `table.clear` is not available until an explicit
`require('table.clear')` call.

> --- a/src/lua/trigger.lua
> +++ b/src/lua/trigger.lua
> @@ -1,4 +1,5 @@
>  local fun = require('fun')
> +local _ = require('table.clear')

I would add a comment that would explain why it is necessary. Just to
don't confuse a reader of the code.

It seems it was added non-intentionally in
8a3dae66f6f2c8dffac39bbda1b38d1e17dfcc63 ('On schema reload callback for
net.box module') (not used anywhere in sources). However now we see that
the membership module leans on the fact that it is available without
explicit 'require'.

I would look for more appropriate place for it: there is nothing special
in trigger.lua. Maybe src/lua/table.lua or src/lua/init.lua. The former
looks better for me.

WBR, Alexander Turenko.