Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: sergeyb@tarantool.org
Cc: o.piskunov@tarantool.org, tarantool-patches@dev.tarantool.org,
	v.shpilevoy@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v6 05/25] Fix luacheck warnings in src/lua/
Date: Mon, 1 Jun 2020 17:47:41 +0300	[thread overview]
Message-ID: <20200601144741.phdbyvvuyjwv6hjo@tkn_work_nb> (raw)
In-Reply-To: <b10824401496b58ef509ad3fc8b029071f88c8b6.1590764167.git.sergeyb@tarantool.org>

> -local function parameters_parse(t_in, options)
> -    local t_out, t_in = {}, t_in or {}
> +local function parameters_parse(param, options)
> +    local t_out, t_in = {}, param or {}

Here you introduce the second name for one thing. It does not improve
the code readability...

>  
>      -- Prepare a lookup table for options. An option name -> a
>      -- type name to convert a parameter into or true (which means
> @@ -145,7 +145,7 @@ local function parameters_parse(t_in, options)
>                  if parameter_has_value(lookup[command]) then
>                      -- in case next argument is value of this key (not --arg)
>                      local next_arg = t_in[i + 1]
> -                    local is_long, is_short, is_dash = parse_param_prefix(next_arg)
> +                    is_long, is_short, is_dash = parse_param_prefix(next_arg)

I don't see any reason to make the live interval of those very local
values longer. The code will be harder to verify.

I never see any positive impact from redefinitions detection in
luacheck, but those examples show that it may push us to introduce
different terms for one thing and to reuse one variable for logically
different values. It would be too pathetic to say that it is harmful,
but it certainly not good. (NB: Also said about this in [1].)

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-June/017304.html

> -local function ibuf_tostring(ibuf)
> +local function ibuf_tostring()
>      return '<ibuf>'
>  end

The function contract assumes that it receives one argument. Whether
luacheck happy about this or not, we should not confuse a reader and
write the function like it does not receive any value.

I propose to use '-- luacheck: no unused args' for those cases, see [2].

[2]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-June/017307.html

WBR, Alexander Turenko.

  parent reply	other threads:[~2020-06-01 14:47 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-29 15:08 [Tarantool-patches] [PATCH v6 00/25] Add static analysis with luacheck sergeyb
2020-05-29 15:08 ` [Tarantool-patches] [PATCH v6 01/25] Add initial luacheck config sergeyb
2020-05-29 16:04   ` Igor Munkin
2020-05-29 16:27     ` Igor Munkin
2020-05-30 12:19       ` Sergey Bronnikov
2020-05-30 12:18     ` Sergey Bronnikov
2020-05-29 15:08 ` [Tarantool-patches] [PATCH v6 02/25] build: enable 'make luacheck' target sergeyb
2020-05-29 16:28   ` Igor Munkin
2020-05-29 15:08 ` [Tarantool-patches] [PATCH v6 03/25] gitlab-ci: enable static analysis with luacheck sergeyb
2020-05-29 19:25   ` Igor Munkin
2020-06-01  9:29     ` Sergey Bronnikov
2020-06-01  9:48       ` Igor Munkin
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 04/25] Fix luacheck warnings in extra/dist/tarantoolctl.in sergeyb
2020-05-29 16:35   ` Igor Munkin
2020-06-01 14:10   ` Alexander Turenko
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 05/25] Fix luacheck warnings in src/lua/ sergeyb
2020-05-29 16:51   ` Igor Munkin
2020-05-29 19:13     ` Igor Munkin
2020-05-30 12:15       ` Sergey Bronnikov
2020-06-01  9:43         ` Igor Munkin
2020-06-01 10:36           ` Sergey Bronnikov
2020-06-01  9:38     ` Sergey Bronnikov
2020-06-01 14:47   ` Alexander Turenko [this message]
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 06/25] Fix luacheck warnings in src/box/lua/ sergeyb
2020-05-29 19:11   ` Igor Munkin
2020-05-30 12:22     ` Sergey Bronnikov
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 07/25] Supress luacheck warnings in test/app sergeyb
2020-06-01 10:11   ` Igor Munkin
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 08/25] Fix luacheck warnings in test/app-tap sergeyb
2020-06-01 11:41   ` Igor Munkin
2020-07-16 11:44     ` Sergey Bronnikov
2020-07-16 12:42       ` Igor Munkin
2020-07-16 13:25         ` Sergey Bronnikov
2020-06-01 13:37   ` Alexander Turenko
2020-06-01 16:37     ` Igor Munkin
2020-06-01 17:13       ` Alexander Turenko
2020-06-01 17:38         ` Igor Munkin
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 09/25] Fix luacheck warnings in test/box sergeyb
2020-06-01 16:06   ` Igor Munkin
2020-07-16 13:23     ` Sergey Bronnikov
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 10/25] Fix luacheck warnings in test/box-py sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 11/25] Fix luacheck warnings in test/box-tap sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 12/25] Fix luacheck warnings in test/engine sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 13/25] Fix luacheck warnings in test/engine_long sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 14/25] Fix luacheck warnings in test/long_run-py sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 15/25] Fix luacheck warnings in test/replication sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 16/25] Fix luacheck warnings in test/replication-py sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 17/25] Fix luacheck warnings in test/sql sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 18/25] Fix luacheck warnings in test/sql-tap sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 19/25] Fix luacheck warnings in test/swim sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 20/25] Fix luacheck warnings in test/vinyl sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 21/25] Fix luacheck warnings in test/wal_off sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 22/25] Fix luacheck warnings in test/xlog sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 23/25] Fix luacheck warnings in test/xlog-py sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 24/25] Add luacheck supressions for luajit tests sergeyb
2020-05-29 15:09 ` [Tarantool-patches] [PATCH v6 25/25] luajit: bump new version sergeyb
2020-06-01 17:08 ` [Tarantool-patches] [PATCH v6 00/25] Add static analysis with luacheck Vladislav Shpilevoy
2020-06-01 17:29   ` Alexander Turenko
2020-06-01 18:13     ` Igor Munkin
2020-06-02 14:42 ` Alexander Turenko
2020-06-02 15:58   ` Igor Munkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200601144741.phdbyvvuyjwv6hjo@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=o.piskunov@tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v6 05/25] Fix luacheck warnings in src/lua/' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox