Hi, Max!

thanks for your review!

 

On 7/17/23 21:48, Maxim Kokryashkin via Tarantool-patches wrote:

Hi, Sergey!

Thanks for the patch!

Please consider my comments below.

 

 

From: Sergey Bronnikov <sergeyb@tarantool.org>

Patch enables running checkpatch [1] for checking patch on a pre-commit

The pre-commit stage makes no sense when we are talking about CI — the

commit has already occurred. That’s just a conventional workflow configuration.

 

In my mind "pre-commit" means commit to a target branch, in our case it is a "tarantool/master".

Having that in mind, along with some grammar related issues. I suggest

rephrasing it like:

| Patch adds a CI workflow with the checkpatch[1] run.

Updated.

stage.

1. https://github.com/tarantool/checkpatch
---
 .github/actions/checkpatch/action.yml | 11 +++++++++++
 .github/workflows/lint.yml | 20 ++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 .github/actions/checkpatch/action.yml

diff --git a/.github/actions/checkpatch/action.yml b/.github/actions/checkpatch/action.yml
new file mode 100644
index 00000000..2336fb15
--- /dev/null
+++ b/.github/actions/checkpatch/action.yml
@@ -0,0 +1,11 @@
+name: Checkpatch
+description: Check patches against LuaJIT development guidelines

I think it’s better to change the action name to `Setup checkpatch`,

since the actual run is not performed here.

The description should be updated correspondingly. Also, AFAIK, there

is no such thing as `LuaJIT guidelines`, so maybe it’s better to change it

to `Tarantool guidelines` if you want to keep that part.

Updated.

 

<snipped>