<HTML><BODY><div>Hi, Sergey!</div><div>Thanks for the patch!</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_16896011841430095759_BODY">From: Sergey Bronnikov <<a href="/compose?To=sergeyb@tarantool.org">sergeyb@tarantool.org</a>><br><br>Patch enables running checkpatch [1] for checking patch on a pre-commit</div></div></div></div></blockquote><div>The pre-commit stage makes no sense when we are talking about CI — the</div><div>commit has already occurred. That’s just a conventional workflow configuration.</div><div> </div><div>Having that in mind, along with some grammar related issues. I suggest</div><div>rephrasing it like:</div><div>| Patch adds a CI workflow with the checkpatch[1] run.</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>stage.<br><br>1. <a href="https://github.com/tarantool/checkpatch" target="_blank">https://github.com/tarantool/checkpatch</a><br>---<br> .github/actions/checkpatch/action.yml | 11 +++++++++++<br> .github/workflows/lint.yml | 20 ++++++++++++++++++++<br> 2 files changed, 31 insertions(+)<br> create mode 100644 .github/actions/checkpatch/action.yml<br><br>diff --git a/.github/actions/checkpatch/action.yml b/.github/actions/checkpatch/action.yml<br>new file mode 100644<br>index 00000000..2336fb15<br>--- /dev/null<br>+++ b/.github/actions/checkpatch/action.yml<br>@@ -0,0 +1,11 @@<br>+name: Checkpatch<br>+description: Check patches against LuaJIT development guidelines</div></div></div></div></blockquote><div>I think it’s better to change the action name to `Setup checkpatch`,</div><div>since the actual run is not performed here.</div><div>The description should be updated correspondingly. Also, AFAIK, there</div><div>is no such thing as `LuaJIT guidelines`, so maybe it’s better to change it</div><div>to `Tarantool guidelines` if you want to keep that part.</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>+runs:<br>+ using: composite<br>+ steps:<br>+ - uses: actions/checkout@v3<br>+ with:<br>+ repository: tarantool/checkpatch<br>+ path: 'checkpatch'<br>+ - run: apt install -y codespell<br>+ shell: bash<br>diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml<br>index 44338f6d..d86c5d54 100644<br>--- a/.github/workflows/lint.yml<br>+++ b/.github/workflows/lint.yml<br>@@ -53,3 +53,23 @@ jobs:<br> - name: luacheck<br> run: cmake --build . --target LuaJIT-luacheck<br> working-directory: ${{ env.BUILDDIR }}<br>+<br>+ checkpatch:<br>+ runs-on: [self-hosted, lightweight, Linux, x86_64]<br>+<br>+ steps:<br>+ - uses: actions/checkout@v3<br>+ with:<br>+ fetch-depth: 0<br>+ submodules: recursive<br>+ - name: checkpatch<br>+ uses: ./.github/actions/checkpatch<br>+ - name: environment<br>+ uses: ./.github/actions/setup<br>+ - name: make tarantool/master available<br>+ run: git checkout tarantool/master && git checkout -<br>+ - name: configure<br>+ run: cmake -S . -B ${{ env.BUILDDIR }}<br>+ - name: checkpatch<br>+ run: cmake --build . --target LuaJIT-checkpatch<br>+ working-directory: ${{ env.BUILDDIR }}<br>--<br>2.34.1</div></div></div></div></blockquote><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div><div> </div></div></blockquote></BODY></HTML>