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.
next prev 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