Hi, Max
thanks for review! see my comments
Hi, Sergey!Please consider my comments below.Среда, 11 октября 2023, 19:54 +03:00 от Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
From: Sergey Bronnikov <sergeyb@tarantool.org>
The patch introduces a new CMake target: "LuaJIT-codespell", that
spellchecks files specified in a whitelist by codespell [1].Typo: s/a whitelist/the whitelist/
Fixed.
1. https://github.com/codespell-project/codespell
---
CMakeLists.txt | 1 +
cmake/CodeSpell.cmake | 36 ++++++++++++++++++++++++++++++++
test/CMakeLists.txt | 1 +
tools/codespell-ignore-words.txt | 3 +++
4 files changed, 41 insertions(+)
create mode 100644 cmake/CodeSpell.cmake
create mode 100644 tools/codespell-ignore-words.txt
diff --git a/CMakeLists.txt b/CMakeLists.txt
index eebf3d6f..7ef10f2f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -32,6 +32,7 @@ set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
include(LuaJITUtils)
include(SetBuildParallelLevel)
include(SetVersion)
+include(CodeSpell)
# --- Variables to be exported to child scopes ---------------------------------
diff --git a/cmake/CodeSpell.cmake b/cmake/CodeSpell.cmake
new file mode 100644
index 00000000..c4d3555d
--- /dev/null
+++ b/cmake/CodeSpell.cmake
@@ -0,0 +1,36 @@
+find_program(CODESPELL codespell)
+
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/lj_mapi.c)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/lj_sysprof.c)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/lj_utils_leb128.c)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/lj_wbuf.c)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/luajit-gdb.py)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/src/luajit_lldb.py)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/test/CMakeLists.txt)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/test/tarantool-c-tests)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/test/tarantool-tests)
+list(APPEND CODESPELL_WHITELIST ${PROJECT_SOURCE_DIR}/tools)CMake’s list is variadic, you can add all entries in one go.
Sure, I know. This way it looks better and changes in patches will be more readable.
+
+set(IGNORE_WORDS ${PROJECT_SOURCE_DIR}/tools/codespell-ignore-words.txt)
+
+add_custom_target(${PROJECT_NAME}-codespell)
+if (CODESPELL)
+ add_custom_command(TARGET ${PROJECT_NAME}-codespell
+ COMMENT "Running codespell"
+ COMMAND
+ ${CODESPELL}
+ --ignore-words ${IGNORE_WORDS}
+ --skip ${IGNORE_WORDS}
+ --ignore-words-list fpr
+ --check-filenames
+ ${CODESPELL_WHITELIST}
+ WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+ )
+else ()
+ set(WARN_MSG "`codespell' is not found, "
+ "so ${PROJECT_NAME}-codespell target is dummy")
+ add_custom_command(TARGET ${PROJECT_NAME}-codespell
+ COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${MSG}
+ COMMENT ${MSG}Why a `message` call with level set to WARNING is not enough?
it is a CMake target, you cannot use `message` command there.
Is there a concernabout coloring?
for me warning highlighted by red color will be more notable than without it.
If so, I believe it’s inconsistent to use approach with cmake_echo_color.In case of `prove` absence on the machine, the more important message aboutskipped regression tests will be less noticeable in comparison with codespell, which isby far less important.
+ )
+endif (CODESPELL)
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 58cba5ba..8afc42df 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -68,6 +68,7 @@ endif()
add_custom_target(${PROJECT_NAME}-lint DEPENDS
${PROJECT_NAME}-luacheck
${PROJECT_NAME}-flake8
+ ${PROJECT_NAME}-codespell
)
set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
diff --git a/tools/codespell-ignore-words.txt b/tools/codespell-ignore-words.txt
new file mode 100644
index 00000000..ceeed47c
--- /dev/null
+++ b/tools/codespell-ignore-words.txt
@@ -0,0 +1,3 @@
+mmaped
+isnt
+FPR
--
2.34.1--
Best regards,Maxim Kokryashkin