<HTML><BODY><div>Hi Sergey, thank you for the patch, please check my comments below.<br><br> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Среда, 8 апреля 2020, 18:43 +03:00 от Sergey Bronnikov <estetus@gmail.com>:<br> <div id=""><div class="js-helper js-readmsg-msg"><style type="text/css"></style><div><div id="style_15863606161136435058_BODY">From: Sergey Bronnikov <<a href="/compose?To=sergeyb@tarantool.org">sergeyb@tarantool.org</a>><br><br>Closes #4681<br>---</div></div></div></div></blockquote></div><div>Please add in the review letter the link to the branch here, also the issue link.</div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div> .gitlab-ci.yml | 9 +++++++++<br> .travis.mk | 12 +++++++++++-<br> 2 files changed, 20 insertions(+), 1 deletion(-)<br><br>diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml<br>index cd710027f..adbd892c8 100644<br>--- a/.gitlab-ci.yml<br>+++ b/.gitlab-ci.yml<br>@@ -1,4 +1,5 @@<br> stages:<br>+ - static_analysis</div></div></div></div></blockquote></div><div>Do we really need the separate stage for this job ? As we agreed with A. Turenko before, we create stages only for jobs that needs additional steps w/o next steps wont work and which can be used in the next stage for more than single job.<br>Anyway separate stage will add depends for the rest of the jobs, which can be blocked because of any issue with luacheck, like it can be luacheck cloning either its depends installations:<div><br>Installing http://rocks.tarantool.org/luacheck-scm-1.rockspec<br>Missing dependencies for luacheck scm-1:<br> argparse >= 0.6.0 (not installed)</div><div>luacheck scm-1 depends on argparse >= 0.6.0 (not installed)<br>Installing http://rocks.tarantool.org/argparse-0.6.0-1.rockspec</div> <div>Cloning into 'luacheck'...<br>remote: Enumerating objects: 42, done.<br>remote: Counting objects: 100% (42/42), done.<br>remote: Compressing objects: 100% (25/25), done.<br>remote: Total 6077 (delta 15), reused 30 (delta 13), pack-reused 6035<br>Receiving objects: 100% (6077/6077), 2.18 MiB | 1.57 MiB/s, done.<br>Resolving deltas: 100% (3980/3980), done.<br>luacheck scm-1 is now installed in /root/tarantool/.rocks (license: MIT)<br> </div></div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div> - test<br> - perf<br> - cleanup<br>@@ -96,6 +97,14 @@ variables:<br> script:<br> - ${GITLAB_MAKE} perf_run<br> <br>+# Static Analysis<br>+<br>+luacheck:<br>+ <<: *docker_test_definition<br>+ stage: static_analysis<br>+ script:<br>+ - ${GITLAB_MAKE} test_debian_luacheck<br>+<br> # Tests<br> <br> release:<br>diff --git a/.travis.mk b/.travis.mk<br>index f709a18b6..32222621b 100644<br>--- a/.travis.mk<br>+++ b/.travis.mk<br>@@ -77,8 +77,9 @@ deps_buster_clang_8: deps_debian<br> # Release<br> <br> build_debian:<br>- cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}<br>+ cmake . -DENABLE_DIST=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}</div></div></div></div></blockquote></div><div>This target is common wide along all the jobs and if ENABLE_DIST define is not adding changes for them may be just need add the comment the purpose on its adding.</div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div> make -j<br>+ sudo make install</div></div></div></div></blockquote></div><div>This target is common wide along all the jobs and may be it better to move the installation to the target ‘luacheck’ which only needs it.</div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div> <br> test_debian_no_deps: build_debian<br> cd test && /usr/bin/python test-run.py --force $(TEST_RUN_EXTRA_PARAMS)<br>@@ -145,6 +146,15 @@ test_static_build: deps_debian_static<br> test_static_docker_build:<br> docker build --network=host --build-arg RUN_TESTS=ON -f Dockerfile.staticbuild .<br> <br>+# ###################<br>+# Static Analysis<br>+# ###################<br>+<br>+test_debian_luacheck: build_debian<br>+ tarantoolctl rocks install luacheck<br>+ # TODO: run in parallel with LuaLanes<br>+ .rocks/bin/luacheck --codes --config .luacheckrc .<br>+<br> #######<br> # OSX #<br> #######<br>--<br>2.23.0<br> </div></div></div></div></blockquote> <div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Alexander Tikhonov</div></div></div><div> </div></div></BODY></HTML>