<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi, Max!</p>
<p>thanks for your review!</p>
<p><br>
</p>
<div class="moz-cite-prefix">On 7/17/23 21:48, Maxim Kokryashkin via
Tarantool-patches wrote:<br>
</div>
<blockquote type="cite"
cite="mid:1689619710.740225619@f193.i.mail.ru">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<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"
moz-do-not-send="true">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>
</blockquote>
</blockquote>
In my mind "pre-commit" means commit to a target branch, in our case
it is a "tarantool/master".<br>
<blockquote type="cite"
cite="mid:1689619710.740225619@f193.i.mail.ru">
<blockquote style="border-left:1px solid #0857A6; margin:10px;
padding:0 0 0 10px;">
<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>
</div>
</blockquote>
</blockquote>
Updated.<br>
<blockquote type="cite"
cite="mid:1689619710.740225619@f193.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>stage.<br>
<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>
---<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>
</div>
</blockquote>
</blockquote>
Updated.<br>
<p><br>
</p>
<p><snipped><br>
</p>
<blockquote type="cite"
cite="mid:1689619710.740225619@f193.i.mail.ru">
<blockquote style="border-left:1px solid #0857A6; margin:10px;
padding:0 0 0 10px;">
<div>
<div> </div>
</div>
</blockquote>
</blockquote>
</body>
</html>