From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id D714D5284A0; Fri, 14 Jul 2023 15:50:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D714D5284A0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1689339005; bh=ioWp+TjLzODGWihpMJhwkxBqycS8VgneKqwiH+4M+6o=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=kJIFChVIYmOIzhckBGOnbw1ewL6WY5GuEj/TTrAIeQOvnaSVg+wCQlcYjBjDn0HdJ q5P7VG0fAYteeBnJRU/1CQnjUCQzdeLnjFc4cS2a/H4Z6JuJ6Y3L47KQkBSiS84vgh 2fnANQBS0C+nFe7CEQmvub6nvqd8N4/tWHhjsglw= Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [95.163.41.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 015A25284A0 for ; Fri, 14 Jul 2023 15:50:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 015A25284A0 Received: by smtp33.i.mail.ru with esmtpa (envelope-from ) id 1qKIFG-006A8g-VC; Fri, 14 Jul 2023 15:50:03 +0300 Date: Fri, 14 Jul 2023 15:45:40 +0300 To: Sergey Bronnikov Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD97569A0FE902DCB3DF525A235365A19A2D5B01E069C4E5BA91867C24CE74E72BB5FE18E9CBFBE7C075CA01661BBF6BEC3FCFDEA389DC56BC913D68621002812FB160237749B4E196D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE788A2BECDB72A1542EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637E6006D770ADD73CF8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8D37BDDDC8D3822F41C20A593F6549B98117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B3238885582065B7B389733CBF5DBD5E9B5C8C57E37DE458BD9DD9810294C998ED8FC6C240DEA76428AA50765F790063764BFA7338FF20B48D81D268191BDAD3DBD4B6F7A4D31EC0BE2F48590F00D11D6D81D268191BDAD3D78DA827A17800CE70C7584EF2E30C7F8EC76A7562686271ED91E3A1F190DE8FD2E808ACE2090B5E14AD6D5ED66289B5259CC434672EE63711DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C369456C5265B6C55C35872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A52A8A2EACDED7547366E02881E1CAD680EBAFFCA96C2C5FDFF87CCE6106E1FC07E67D4AC08A07B9B0DB8A315C1FF4794DBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFB695074CDD25C268DA657DD5ED1CE1A5B6248D849AA5597D3F9F0116E2B0EC1A9901B20BD73D67EE7BBDB0A562C82D5943C362EDDAFAFD829768C7E6CEF08EC8A74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojrC/lX9ZwUYpkbGItmFo/YA== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769FF83E92B574D72DD7CF116D65ADF2EE8C59A7EFB72EB47B2DEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 2/2] ci: enable checkpatch X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the patch! Please, consider my comments below. On 11.07.23, Sergey Bronnikov wrote: > From: Sergey Bronnikov > > Patch enables checkpatch [1] for checking patch on a pre-commit stage. Minor: Strictly saying it's not a precommit stage (since it's not a pre-commit hook). But this is a prerequisite for other testing workflow. > In Tarantool we use our own fork of checkpatch [2] with additional check > types. It's logical to use it in a LuaJIT development. We don't need Also, it is good to have some checks for our C code like: * * * * * * * * * But they are contradicting with Tarantool's guidelines as far as they are written in LuaJIT's style. Have we some way to check them too? > check tags in commit messages like NO_DOC, NO_CHANGELOG, NO_TEST and Side note: Not so sure, that NO_TEST check is a good thing, but I'm OK with it, since it may be useful for backporting refactoring changes. > others, so to be able to customize command-line options Github Action, provided > by checkpatch repository [3], was added to the repository. > > See documentation for used checkpatch in [4]. > > 1. https://docs.kernel.org/dev-tools/checkpatch.html > 2. https://github.com/tarantool/checkpatch > 3. https://github.com/tarantool/checkpatch/blob/master/.github/actions/checkpatch/action.yml > 4. https://github.com/tarantool/checkpatch/blob/master/doc/checkpatch.rst Also, is it possible to create the make target similar to LuaJIT-luacheck? It should: 0) Be dummy if there is no chekcpatch or codespell installed. 1) Be included in the make test check. It is useful for local spellcheck without any push to remote CI. > --- > .github/actions/checkpatch/action.yml | 17 +++++++++++++++++ > .github/workflows/lint.yml | 14 ++++++++++++++ > 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..df2e2a2b > --- /dev/null > +++ b/.github/actions/checkpatch/action.yml > @@ -0,0 +1,17 @@ > +name: Checkpatch > +description: Check patches against LuaJIT development guidelines > +inputs: > + revision-range: > + description: Git revision range to check > + required: true > +runs: > + using: composite > + steps: > + - uses: actions/checkout@v3 > + with: > + repository: tarantool/checkpatch > + path: 'checkpatch' > + - run: apt install -y codespell > + shell: bash > + - run: checkpatch/checkpatch.pl --codespell --color=always --show-types --git ${{ inputs.revision-range }} --codespellfile /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt --ignore NO_CHANGELOG,NO_DOC,NO_TEST,COMMIT_LOG_LONG_LINE Please, split this line into several. > + shell: bash > diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml > index 71ceee9a..d28aa15b 100644 > --- a/.github/workflows/lint.yml > +++ b/.github/workflows/lint.yml > @@ -53,3 +53,17 @@ jobs: > - name: test > run: cmake --build . --target LuaJIT-luacheck > working-directory: ${{ env.BUILDDIR }} > + > + checkpatch: > + runs-on: [self-hosted, lightweight, Linux, x86_64] > + > + steps: > + - uses: actions/checkout@v3 > + with: > + fetch-depth: 0 > + # ref: ${{ github.event.pull_request.head.sha }} Minor: Why do we need this comment? > + submodules: recursive > + - name: checkpatch > + uses: ./.github/actions/checkpatch > + with: > + revision-range: HEAD~${{ github.event.pull_request.commits }}..HEAD There is something wrong with this definition; in CI [1] it changes to the following: | checkpatch/checkpatch.pl --codespell --color=always --show-types --git HEAD~..HEAD --ignore NO_CHANGELOG,NO_DOC,NO_TEST,COMMIT_LOG_LONG_LINE So, only the top commit is verified (instead of 2 -- this is why there is a typo in the previous commit message). Also, is it possible to do this activity on push instead, depending on difference with tarantool/master HEAD? Also, how can we avoid some errors here, if this will show incorrect spelling in the backported patches? > -- > 2.34.1 > [1]: https://github.com/tarantool/luajit/actions/runs/5519618350/jobs/10065116711#step:3:143 -- Best regards, Sergey Kaplun