<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi, Max<br>
</p>
<div class="moz-cite-prefix">On 8/3/23 22:38, Maxim Kokryashkin via
Tarantool-patches wrote:<br>
</div>
<blockquote type="cite"
cite="mid:1691091489.404344804@f148.i.mail.ru">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div>Hi, Sergey!</div>
<div>Please consider my comments below.</div>
<div> </div>
<blockquote style="border-left:1px solid #0857A6; margin:10px;
padding:0 0 0 10px;">
<div>
<blockquote style="border-left:1px solid #0857A6; margin:10px;
padding:0 0 0 10px;">
<div id="">
<div class="js-helper js-readmsg-msg">
<div>
<div id="style_16909666541899939236_BODY">From: Sergey
Bronnikov <<a
href="/compose?To=sergeyb@tarantool.org"
moz-do-not-send="true">sergeyb@tarantool.org</a>><br>
<br>
In Tarantool we use our own fork of checkpatch [1]
with additional check<br>
types. It's logical to use it in LuaJIT development.
However, we don't<br>
need to enable all checks [2] implemented in
checkpatch, therefore a<br>
number of checks are disabled.<br>
<br>
Patch introduces two new CMake targets:
"LuaJIT-checkpatch", that checks<br>
patches on top of the master branch using script
checkpatch.pl, and</div>
</div>
</div>
</div>
</blockquote>
<div>Typo: s/using/using the/</div>
</div>
</blockquote>
</blockquote>
<p>Fixed.</p>
<blockquote type="cite"
cite="mid:1691091489.404344804@f148.i.mail.ru">
<blockquote style="border-left:1px solid #0857A6; margin:10px;
padding:0 0 0 10px;">
<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>target "check", that combines LuaJIT-luacheck and
LuaJIT-checkpatch. By<br>
default CMake looking for checkpatch.pl in a
directory "checkpatch" in</div>
</div>
</div>
</div>
</blockquote>
<div>Typo: s/looking/looks/</div>
<div>Typo: s/in a directory/in the directory/</div>
</div>
</blockquote>
</blockquote>
Fixed.<br>
<blockquote type="cite"
cite="mid:1691091489.404344804@f148.i.mail.ru">
<blockquote style="border-left:1px solid #0857A6; margin:10px;
padding:0 0 0 10px;">
<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>LuaJIT repository root directory and in a
directories specified in PATH.</div>
</div>
</div>
</div>
</blockquote>
<div>Typo: s/LuaJIT/the LuaJIT/</div>
<div>Typo: s/a directories/directories/</div>
</div>
</blockquote>
</blockquote>
<p>Fixed.</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:1691091489.404344804@f148.i.mail.ru">
<blockquote style="border-left:1px solid #0857A6; margin:10px;
padding:0 0 0 10px;">
<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><br>
1. <a
href="https://github.com/tarantool/checkpatch"
target="_blank" moz-do-not-send="true"
class="moz-txt-link-freetext">https://github.com/tarantool/checkpatch</a><br>
2. <a
href="https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst"
target="_blank" moz-do-not-send="true"
class="moz-txt-link-freetext">https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst</a><br>
---<br>
test/CMakeLists.txt | 51
+++++++++++++++++++++++++++++++++++++++++++++<br>
1 file changed, 51 insertions(+)<br>
<br>
diff --git a/test/CMakeLists.txt
b/test/CMakeLists.txt<br>
index 47296a22..5ec0bed6 100644<br>
--- a/test/CMakeLists.txt<br>
+++ b/test/CMakeLists.txt<br>
@@ -42,6 +42,56 @@ else()<br>
)<br>
endif()<br>
<br>
+find_program(CHECKPATCH checkpatch.pl<br>
+ HINTS ${PROJECT_SOURCE_DIR}/checkpatch)<br>
+add_custom_target(${PROJECT_NAME}-checkpatch)<br>
+set(MASTER_BRANCH "tarantool/master")<br>
+if(CHECKPATCH)<br>
+ add_custom_command(TARGET
${PROJECT_NAME}-checkpatch<br>
+ COMMENT "Running checkpatch"<br>
+ COMMAND<br>
+ ${CHECKPATCH}<br>
+ # Description of supported checks in<br>
+ # <a
href="https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst"
target="_blank" moz-do-not-send="true"
class="moz-txt-link-freetext">https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst</a><br>
+ --codespell<br>
+ --color=always<br>
+ --git ${MASTER_BRANCH}..HEAD<br>
+ --show-types<br>
+ --ignore BAD_SIGN_OFF<br>
+ --ignore BLOCK_COMMENT_STYLE<br>
+ --ignore CODE_INDENT<br>
+ --ignore COMMIT_LOG_LONG_LINE<br>
+ # Requires at least two lines in commit message
and this<br>
+ # is annoying.<br>
+ --ignore COMMIT_MESSAGE<br>
+ --ignore CONSTANT_COMPARISON<br>
+ --ignore FUNCTION_NAME_NO_NEWLINE<br>
+ --ignore GIT_COMMIT_ID<br>
+ --ignore INCLUDE_GUARD<br>
+ --ignore LOGICAL_CONTINUATIONS<br>
+ --ignore LONG_LINE<br>
+ --ignore NO_CHANGELOG<br>
+ --ignore NO_DOC<br>
+ --ignore NO_TEST<br>
+ --ignore PREFER_DEFINED_ATTRIBUTE_MACRO<br>
+ --ignore SPACING<br>
+ --ignore SUSPECT_CODE_INDENT<br>
+ --ignore TABSTOP<br>
+ --ignore TRAILING_STATEMENTS<br>
+ --ignore UNCOMMENTED_DEFINITION<br>
+ --ignore UNSAFE_FUNCTION<br>
+ WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}<br>
+ )<br>
+else()</div>
</div>
</div>
</div>
</blockquote>
<div>I suggest doing the same as with coverage — move that
logic into a separate module,</div>
<div>so the test/CMakeLists.txt remains readable and clean.</div>
</div>
</blockquote>
</blockquote>
<p><br>
</p>
<p>Done, force-pushed.</p>
<p><br>
</p>
<p>commit 609f893ecc5ee9895916c637c145489fe59d9bb0<br>
Author: Sergey Bronnikov <a class="moz-txt-link-rfc2396E" href="mailto:estetus@gmail.com"><estetus@gmail.com></a><br>
Date: Fri Aug 4 13:51:23 2023 +0300<br>
<br>
cmake: checkpatch module<br>
<br>
diff --git a/CMakeLists.txt b/CMakeLists.txt<br>
index 0204e852..b1a442b7 100644<br>
--- a/CMakeLists.txt<br>
+++ b/CMakeLists.txt<br>
@@ -29,9 +29,12 @@ endif()<br>
<br>
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")<br>
<br>
+set(GIT_MASTER_BRANCH "tarantool/master")<br>
+<br>
include(LuaJITUtils)<br>
include(SetBuildParallelLevel)<br>
include(SetVersion)<br>
+include(CheckPatch)<br>
<br>
# --- Variables to be exported to child scopes
---------------------------------<br>
<br>
diff --git a/cmake/CheckPatch.cmake b/cmake/CheckPatch.cmake<br>
new file mode 100644<br>
index 00000000..243ee426<br>
--- /dev/null<br>
+++ b/cmake/CheckPatch.cmake<br>
@@ -0,0 +1,46 @@<br>
+find_program(CHECKPATCH checkpatch.pl<br>
+ HINTS ${PROJECT_SOURCE_DIR}/checkpatch)<br>
+add_custom_target(${PROJECT_NAME}-checkpatch)<br>
+if(CHECKPATCH)<br>
+ add_custom_command(TARGET ${PROJECT_NAME}-checkpatch<br>
+ COMMENT "Running checkpatch"<br>
+ COMMAND<br>
+ ${CHECKPATCH}<br>
+ # Description of supported checks in<br>
+ #
<a class="moz-txt-link-freetext" href="https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst">https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst</a><br>
+ --codespell<br>
+ --color=always<br>
+ --git ${GIT_MASTER_BRANCH}..HEAD<br>
+ --show-types<br>
+ --ignore BAD_SIGN_OFF<br>
+ --ignore BLOCK_COMMENT_STYLE<br>
+ --ignore CODE_INDENT<br>
+ --ignore COMMIT_LOG_LONG_LINE<br>
+ # Requires at least two lines in commit message and this<br>
+ # is annoying.<br>
+ --ignore COMMIT_MESSAGE<br>
+ --ignore CONSTANT_COMPARISON<br>
+ --ignore FUNCTION_NAME_NO_NEWLINE<br>
+ --ignore GIT_COMMIT_ID<br>
+ --ignore INCLUDE_GUARD<br>
+ --ignore LOGICAL_CONTINUATIONS<br>
+ --ignore LONG_LINE<br>
+ --ignore NO_CHANGELOG<br>
+ --ignore NO_DOC<br>
+ --ignore NO_TEST<br>
+ --ignore PREFER_DEFINED_ATTRIBUTE_MACRO<br>
+ --ignore SPACING<br>
+ --ignore SUSPECT_CODE_INDENT<br>
+ --ignore TABSTOP<br>
+ --ignore TRAILING_STATEMENTS<br>
+ --ignore UNCOMMENTED_DEFINITION<br>
+ --ignore UNSAFE_FUNCTION<br>
+ WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}<br>
+ )<br>
+else()<br>
+ set(MSG "`checkpatch.pl' is not found, so
${PROJECT_NAME}-checkpatch target is dummy")<br>
+ add_custom_command(TARGET ${PROJECT_NAME}-checkpatch<br>
+ COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${MSG}<br>
+ COMMENT ${MSG}<br>
+ )<br>
+endif()<br>
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt<br>
index 5ec0bed6..66f6ee77 100644<br>
--- a/test/CMakeLists.txt<br>
+++ b/test/CMakeLists.txt<br>
@@ -42,52 +42,6 @@ else()<br>
)<br>
endif()<br>
<br>
-find_program(CHECKPATCH checkpatch.pl<br>
- HINTS ${PROJECT_SOURCE_DIR}/checkpatch)<br>
-add_custom_target(${PROJECT_NAME}-checkpatch)<br>
-set(MASTER_BRANCH "tarantool/master")<br>
-if(CHECKPATCH)<br>
- add_custom_command(TARGET ${PROJECT_NAME}-checkpatch<br>
- COMMENT "Running checkpatch"<br>
- COMMAND<br>
- ${CHECKPATCH}<br>
- # Description of supported checks in<br>
- #
<a class="moz-txt-link-freetext" href="https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst">https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst</a><br>
- --codespell<br>
- --color=always<br>
- --git ${MASTER_BRANCH}..HEAD<br>
- --show-types<br>
- --ignore BAD_SIGN_OFF<br>
- --ignore BLOCK_COMMENT_STYLE<br>
- --ignore CODE_INDENT<br>
- --ignore COMMIT_LOG_LONG_LINE<br>
- # Requires at least two lines in commit message and this<br>
- # is annoying.<br>
- --ignore COMMIT_MESSAGE<br>
- --ignore CONSTANT_COMPARISON<br>
- --ignore FUNCTION_NAME_NO_NEWLINE<br>
- --ignore GIT_COMMIT_ID<br>
- --ignore INCLUDE_GUARD<br>
- --ignore LOGICAL_CONTINUATIONS<br>
- --ignore LONG_LINE<br>
- --ignore NO_CHANGELOG<br>
- --ignore NO_DOC<br>
- --ignore NO_TEST<br>
- --ignore PREFER_DEFINED_ATTRIBUTE_MACRO<br>
- --ignore SPACING<br>
- --ignore SUSPECT_CODE_INDENT<br>
- --ignore TABSTOP<br>
- --ignore TRAILING_STATEMENTS<br>
- --ignore UNCOMMENTED_DEFINITION<br>
- --ignore UNSAFE_FUNCTION<br>
- WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}<br>
- )<br>
-else()<br>
- add_custom_command(TARGET ${PROJECT_NAME}-checkpatch<br>
- COMMENT "`checkpatch.pl' is not found, so
${PROJECT_NAME}-checkpatch target is dummy"<br>
- )<br>
-endif()<br>
-<br>
add_custom_target(check<br>
DEPENDS ${PROJECT_NAME}-checkpatch ${PROJECT_NAME}-luacheck<br>
)<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:1691091489.404344804@f148.i.mail.ru">
<blockquote style="border-left:1px solid #0857A6; margin:10px;
padding:0 0 0 10px;">
<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>+ add_custom_command(TARGET
${PROJECT_NAME}-checkpatch<br>
+ COMMENT "`checkpatch.pl' is not found, so
${PROJECT_NAME}-checkpatch target is dummy"<br>
+ )<br>
+endif()<br>
+<br>
+add_custom_target(check<br>
+ DEPENDS ${PROJECT_NAME}-checkpatch
${PROJECT_NAME}-luacheck<br>
+)<br>
+<br>
set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e
dofile[[${LUAJIT_TEST_INIT}]]")<br>
separate_arguments(LUAJIT_TEST_COMMAND)<br>
<br>
@@ -75,5 +125,6 @@ if(LUAJIT_USE_TEST)<br>
add_custom_target(test DEPENDS<br>
${PROJECT_NAME}-test<br>
${PROJECT_NAME}-luacheck<br>
+ ${PROJECT_NAME}-checkpatch<br>
)<br>
endif()<br>
--<br>
2.34.1</div>
</div>
</div>
</div>
</blockquote>
<div>
<div>--<br>
Best regards,</div>
<div>Maxim Kokryashkin</div>
</div>
</div>
</blockquote>
</blockquote>
</body>
</html>