From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <sergeyb@tarantool.org>
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
 (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 6007845C304
 for <tarantool-patches@dev.tarantool.org>;
 Wed,  9 Dec 2020 09:13:18 +0300 (MSK)
References: <9fa9abe33284ca30177a0642fdd042b5155578e9.1605889896.git.artemreyt@tarantool.org>
 <cfb7232e-111d-275e-c4dd-70357d299ae8@tarantool.org>
 <d3abbdbd-54d2-7f32-6bd0-8e46d0a1f910@tarantool.org>
From: Sergey Bronnikov <sergeyb@tarantool.org>
Message-ID: <0a511e3b-b103-e619-7a90-a31e2164ac17@tarantool.org>
Date: Wed, 9 Dec 2020 09:13:17 +0300
MIME-Version: 1.0
In-Reply-To: <d3abbdbd-54d2-7f32-6bd0-8e46d0a1f910@tarantool.org>
Content-Type: multipart/alternative;
 boundary="------------84C07F59A0031F01F98FCEF1"
Content-Language: en-US
Subject: Re: [Tarantool-patches] [PATCH v2] luacheck: change global vars to
	local in sql-tap
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Artem <artemreyt@tarantool.org>, tml <tarantool-patches@dev.tarantool.org>

This is a multi-part message in MIME format.
--------------84C07F59A0031F01F98FCEF1
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Transfer-Encoding: 8bit

Hello, Artem!

LGTM

On 26.11.2020 15:19, Artem wrote:
>
> Sergey, thanks for the review!
>
> 24.11.2020 21:57, Sergey Bronnikov пишет:
>>
>> Hi, Artem!
>>
>> thanks for the patch! See 2 comment below.
>>
>> On 20.11.2020 19:34, Artem Starshov wrote:
>>> Fixed luacheck warning 111 (setting non-standard global variable)
>>> in test/sql-tap directory.
>>> Enabled this directory for checking W111 in
>>> config file(.luacheckrc).
>>>
>>> Changed almost all variables in test/sql-tap from globals
>>> to locals. In any cases, where variables need to be global,
>>> added inline comments for luacheck.
>>>
>>> Fixes #5173
>>> Part-of #5464
>>> ---
>>
>> Please don't forget to place a branch name, issue number.
>>
>> I believe both are the same as in previous mail:
>>
>> >> Branch: 
>> https://github.com/tarantool/tarantool/tree/artemreyt/gh-5173-fix-luacheck-warnings-sql-tap 
>> >> Issue: https://github.com/tarantool/tarantool/issues/5173
>>
>> <snipped>
>>
>>> diff --git a/.luacheckrc b/.luacheckrc
>>> index 2a652cd06..174bfdf25 100644
>>> --- a/.luacheckrc
>>> +++ b/.luacheckrc
>>> @@ -31,12 +31,67 @@ exclude_files = {
>>>       -- Third-party source code.
>>>       "src/box/lua/serpent.lua",
>>>       "test-run/**/*.lua",
>>> -    "test/**/*.lua",
>>> +    "test/app/**/*.lua",
>>> +    "test/app-tap/**/*.lua",
>>> +    "test/box/**/*.lua",
>>> +    "test/box-py/**/*.lua",
>>> +    "test/box-tap/**/*.lua",
>>> +    "test/engine/**/*.lua",
>>> +    "test/engine_long/**/*.lua",
>>> +    "test/long_run-py/**/*.lua",
>>> +    "test/luajit-tap/**/*.lua",
>>> +    "test/replication/**/*.lua",
>>> +    "test/replication-py/**/*.lua",
>>> +    "test/sql/**/*.lua",
>>> +    "test/swim/**/*.lua",
>>> +    "test/var/**/*.lua",
>>> +    "test/vinyl/**/*.lua",
>>> +    "test/wal_off/**/*.lua",
>>> +    "test/xlog/**/*.lua",
>>> +    "test/xlog-py/**/*.lua",
>>>       "third_party/**/*.lua",
>>>       ".rocks/**/*.lua",
>>>       ".git/**/*.lua",
>>>   }
>>>   
>>> +--
>>> +-- gh-5464: Fix luacheck warnings in test/sql-tap
>>> +--
> 1. Removed on the branch.
>> 1. Number of ticket that will be closed after merging this patch is 
>> useless here.
>>> +files["test/sql-tap/**/*.lua"] = {
>>> +    ignore = {
>>> +        -- Accessing an undefined global variable.
>>> +        "113",
>>> +        -- Unused local variable.
>>> +        "211",
>>> +        -- Unused argument.
>>> +        "212",
>>> +        -- Unused loop variable.
>>> +        "213",
>>> +        -- Local variable is set but never accessed.
>>> +        "231",
>>> +        -- "Value assigned to a local variable is unused."
>>> +        "311",
>>> +        -- Unreachable code.
>>> +        "511",
>>> +        -- Loop can be executed at most once.
>>> +        "512",
>>> +        -- An empty if branch.
>>> +        "542",
>>> +        -- A line consists of nothing but whitespace.
>>> +        "611",
>>> +        -- A line contains trailing whitespace.
>>> +        "612",
>>> +        -- Trailing whitespace in a string.
>>> +        "613",
>>> +        -- Trailing whitespace in a comment.
>>> +        "614",
>>> +        -- Inconsistent indentation (SPACE followed by TAB).
>>> +        "621",
>>> +        -- Line is too long.
>>> +        "631"
>>> +    }
>>> +}
>>> +
>>>   files["src/lua/help.lua"] = {
>>>       -- Globals defined for interactive mode.
>>>       globals = {"help", "tutorial"},
>>> diff --git a/test/sql-tap/aggnested.test.lua b/test/sql-tap/aggnested.test.lua
>>> index 67a9ba891..8a79e0df2 100755
>>> --- a/test/sql-tap/aggnested.test.lua
>>> +++ b/test/sql-tap/aggnested.test.lua
>>> @@ -1,5 +1,5 @@
>>>   #!/usr/bin/env tarantool
>>> -test = require("sqltester")
>>> +local test = require("sqltester")
>>>   test:plan(7)
>>>   
>>>   --!./tcltestrunner.lua
>>> diff --git a/test/sql-tap/alias.test.lua b/test/sql-tap/alias.test.lua
>>> index 75391b305..8181b5ed5 100755
>>> --- a/test/sql-tap/alias.test.lua
>>> +++ b/test/sql-tap/alias.test.lua
>>> @@ -1,5 +1,5 @@
>>>   #!/usr/bin/env tarantool
>>> -test = require("sqltester")
>>> +local test = require("sqltester")
>>>   test:plan(9)
>>>   
>>>   --!./tcltestrunner.lua
>>> @@ -24,6 +24,7 @@ test:plan(9)
>>>   -- A procedure to return a sequence of increasing integers.
>>>   --
>>>   
>>> +-- luacheck: globals counter
>>>   counter = 0
>>>   
>
> 2. It's a bug, created a ticket here:
>
> https://github.com/mpeterv/luacheck/issues/210
>
>> 2. In [1] I asked to place supressions on the same line with code
>>
>> 1. 
>> https://lists.tarantool.org/pipermail/tarantool-patches/2020-November/020606.html
>>
>> <snipped> 

--------------84C07F59A0031F01F98FCEF1
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Hello, Artem!</p>
    <p>LGTM<br>
    </p>
    <div class="moz-cite-prefix">On 26.11.2020 15:19, Artem wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:d3abbdbd-54d2-7f32-6bd0-8e46d0a1f910@tarantool.org">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <p>Sergey, thanks for the review!<br>
      </p>
      <div class="moz-cite-prefix">24.11.2020 21:57, Sergey Bronnikov
        пишет:<br>
      </div>
      <blockquote type="cite"
        cite="mid:cfb7232e-111d-275e-c4dd-70357d299ae8@tarantool.org">
        <meta http-equiv="Content-Type" content="text/html;
          charset=UTF-8">
        <p>Hi, Artem!</p>
        <p>thanks for the patch! See 2 comment below.<br>
        </p>
        <div class="moz-cite-prefix">On 20.11.2020 19:34, Artem Starshov
          wrote:<br>
        </div>
        <blockquote type="cite"
cite="mid:9fa9abe33284ca30177a0642fdd042b5155578e9.1605889896.git.artemreyt@tarantool.org">
          <pre class="moz-quote-pre" wrap="">Fixed luacheck warning 111 (setting non-standard global variable)
in test/sql-tap directory.
Enabled this directory for checking W111 in
config file(.luacheckrc).

Changed almost all variables in test/sql-tap from globals
to locals. In any cases, where variables need to be global,
added inline comments for luacheck.

Fixes #5173
Part-of #5464
---</pre>
        </blockquote>
        <p>Please don't forget to place a branch name, issue number.</p>
        <p>I believe both are the same as in previous mail:</p>
        <p style="white-space: pre-wrap; color: rgb(0, 0, 0); font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;">&gt;&gt; Branch: <a href="https://github.com/tarantool/tarantool/tree/artemreyt/gh-5173-fix-luacheck-warnings-sql-tap" moz-do-not-send="true">https://github.com/tarantool/tarantool/tree/artemreyt/gh-5173-fix-luacheck-warnings-sql-tap</a>
&gt;&gt; Issue: <a href="https://github.com/tarantool/tarantool/issues/5173" moz-do-not-send="true">https://github.com/tarantool/tarantool/issues/5173</a>
</p>
        <p style="white-space: pre-wrap; color: rgb(0, 0, 0); font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;">&lt;snipped&gt;</p>
        <blockquote type="cite"
cite="mid:9fa9abe33284ca30177a0642fdd042b5155578e9.1605889896.git.artemreyt@tarantool.org">
          <pre class="moz-quote-pre" wrap="">diff --git a/.luacheckrc b/.luacheckrc
index 2a652cd06..174bfdf25 100644
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -31,12 +31,67 @@ exclude_files = {
     -- Third-party source code.
     "src/box/lua/serpent.lua",
     "test-run/**/*.lua",
-    "test/**/*.lua",
+    "test/app/**/*.lua",
+    "test/app-tap/**/*.lua",
+    "test/box/**/*.lua",
+    "test/box-py/**/*.lua",
+    "test/box-tap/**/*.lua",
+    "test/engine/**/*.lua",
+    "test/engine_long/**/*.lua",
+    "test/long_run-py/**/*.lua",
+    "test/luajit-tap/**/*.lua",
+    "test/replication/**/*.lua",
+    "test/replication-py/**/*.lua",
+    "test/sql/**/*.lua",
+    "test/swim/**/*.lua",
+    "test/var/**/*.lua",
+    "test/vinyl/**/*.lua",
+    "test/wal_off/**/*.lua",
+    "test/xlog/**/*.lua",
+    "test/xlog-py/**/*.lua",
     "third_party/**/*.lua",
     ".rocks/**/*.lua",
     ".git/**/*.lua",
 }
 
+--
+-- gh-5464: Fix luacheck warnings in test/sql-tap
+--</pre>
        </blockquote>
      </blockquote>
      1. Removed on the branch.<br>
      <blockquote type="cite"
        cite="mid:cfb7232e-111d-275e-c4dd-70357d299ae8@tarantool.org">
        <blockquote type="cite"
cite="mid:9fa9abe33284ca30177a0642fdd042b5155578e9.1605889896.git.artemreyt@tarantool.org">
        </blockquote>
        1. Number of ticket that will be closed after merging this patch
        is useless here.<br>
        <blockquote type="cite"
cite="mid:9fa9abe33284ca30177a0642fdd042b5155578e9.1605889896.git.artemreyt@tarantool.org">
          <pre class="moz-quote-pre" wrap="">+files["test/sql-tap/**/*.lua"] = {
+    ignore = {
+        -- Accessing an undefined global variable.
+        "113",
+        -- Unused local variable.
+        "211",
+        -- Unused argument.
+        "212",
+        -- Unused loop variable.
+        "213",
+        -- Local variable is set but never accessed.
+        "231",
+        -- "Value assigned to a local variable is unused."
+        "311",
+        -- Unreachable code.
+        "511",
+        -- Loop can be executed at most once.
+        "512",
+        -- An empty if branch.
+        "542",
+        -- A line consists of nothing but whitespace.
+        "611",
+        -- A line contains trailing whitespace.
+        "612",
+        -- Trailing whitespace in a string.
+        "613",
+        -- Trailing whitespace in a comment.
+        "614",
+        -- Inconsistent indentation (SPACE followed by TAB).
+        "621",
+        -- Line is too long.
+        "631"
+    }
+}
+
 files["src/lua/help.lua"] = {
     -- Globals defined for interactive mode.
     globals = {"help", "tutorial"},
diff --git a/test/sql-tap/aggnested.test.lua b/test/sql-tap/aggnested.test.lua
index 67a9ba891..8a79e0df2 100755
--- a/test/sql-tap/aggnested.test.lua
+++ b/test/sql-tap/aggnested.test.lua
@@ -1,5 +1,5 @@
 #!/usr/bin/env tarantool
-test = require("sqltester")
+local test = require("sqltester")
 test:plan(7)
 
 --!./tcltestrunner.lua
diff --git a/test/sql-tap/alias.test.lua b/test/sql-tap/alias.test.lua
index 75391b305..8181b5ed5 100755
--- a/test/sql-tap/alias.test.lua
+++ b/test/sql-tap/alias.test.lua
@@ -1,5 +1,5 @@
 #!/usr/bin/env tarantool
-test = require("sqltester")
+local test = require("sqltester")
 test:plan(9)
 
 --!./tcltestrunner.lua
@@ -24,6 +24,7 @@ test:plan(9)
 -- A procedure to return a sequence of increasing integers.
 --
 
+-- luacheck: globals counter
 counter = 0
 </pre>
        </blockquote>
      </blockquote>
      <p>2. It's a bug, created a ticket here:</p>
      <p><a class="moz-txt-link-freetext"
          href="https://github.com/mpeterv/luacheck/issues/210"
          moz-do-not-send="true">https://github.com/mpeterv/luacheck/issues/210</a><br>
      </p>
      <blockquote type="cite"
        cite="mid:cfb7232e-111d-275e-c4dd-70357d299ae8@tarantool.org">
        <blockquote type="cite"
cite="mid:9fa9abe33284ca30177a0642fdd042b5155578e9.1605889896.git.artemreyt@tarantool.org">
        </blockquote>
        <p>2. In [1] I asked to place supressions on the same line with
          code</p>
        <p>1. <a class="moz-txt-link-freetext"
href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-November/020606.html"
            moz-do-not-send="true">https://lists.tarantool.org/pipermail/tarantool-patches/2020-November/020606.html</a><br>
        </p>
        &lt;snipped&gt; </blockquote>
    </blockquote>
  </body>
</html>

--------------84C07F59A0031F01F98FCEF1--