From: Leonid Vasiliev <lvasiliev@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, alexander.turenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH 1/2] luarocks: add a kludge for option all of luarocks search Date: Mon, 16 Mar 2020 14:58:46 +0300 [thread overview] Message-ID: <8d2fd71d-fcf3-5653-7bf9-0420523ac3c7@tarantool.org> (raw) In-Reply-To: <bab829cb-45df-efee-8c62-45d3a903e24c@tarantool.org> Hi. Thanks for the review! On 3/15/20 8:54 PM, Vladislav Shpilevoy wrote: > Thanks for the patch! > > Please, provide a more detailed message of what and why was > done, exactly. And how to test it. And a docbot request, if > you change any publicly visible behaviour of a documented > module. Behavior has't been changed, just fixed. > > All the same for the second patch. If you will add a docbot > request, it is enough to make one for two commits. > > Also, please, rebase the branch on the latest master. > Done. See V2 > See 2 comments below. > > On 15/11/2019 16:44, Leonid wrote: >> --- >> extra/dist/tarantoolctl.in | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in >> index 6daf866ac..09302ea9c 100755 >> --- a/extra/dist/tarantoolctl.in >> +++ b/extra/dist/tarantoolctl.in >> @@ -937,6 +937,7 @@ local function rocks() >> local util = require("luarocks.util") >> local command_line = require("luarocks.cmd") >> local options = keyword_arguments >> + > > 1. Please, don't do stray changes not related to the issues. > >> local key = nil >> if options["only-server"] ~= nil then >> key = find_arg("only%-server") >> @@ -945,6 +946,10 @@ local function rocks() >> end >> table.insert(positional_arguments, key) >> >> + if positional_arguments[1] == "search" and find_arg("all") then > > 2. Double whitespace after 'and'. The same in the next patch. > >> + table.insert(positional_arguments, "--all") >> + end >> + >> -- Tweak help messages >> util.see_help = function(command, program) >> -- TODO: print extended help message here >> @@ -1400,6 +1405,7 @@ local function populate_arguments() >> { 'only-server', 'string' }, >> { 'server', 'string' }, >> { 'language', 'string' }, >> + { 'all', 'boolean' }, >> }) >> >> local cmd_name >>
next prev parent reply other threads:[~2020-03-16 11:58 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-15 15:44 [Tarantool-patches] [PATCH 0/2] Fix some options for rocks Leonid 2019-11-15 15:44 ` [Tarantool-patches] [PATCH 1/2] luarocks: add a kludge for option all of luarocks search Leonid 2020-03-15 17:54 ` Vladislav Shpilevoy 2020-03-16 11:58 ` Leonid Vasiliev [this message] 2019-11-15 15:44 ` [Tarantool-patches] [PATCH 2/2] luarocks: add a kludge for option force of luarocks remove Leonid 2020-03-15 17:54 ` [Tarantool-patches] [PATCH 0/2] Fix some options for rocks Vladislav Shpilevoy
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=8d2fd71d-fcf3-5653-7bf9-0420523ac3c7@tarantool.org \ --to=lvasiliev@tarantool.org \ --cc=alexander.turenko@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/2] luarocks: add a kludge for option all of luarocks search' \ /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