Tarantool development patches archive
 help / color / mirror / Atom feed
From: lvasiliev <lvasiliev@tarantool.org>
To: Yaroslav Dynnikov <yaroslav.dynnikov@tarantool.org>,
	Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl
Date: Fri, 10 Apr 2020 21:37:27 +0300	[thread overview]
Message-ID: <6b4973cb-7c9e-3f18-c55d-12e30821da2e@tarantool.org> (raw)
In-Reply-To: <CAK0MaD27MC9NSKi8_8eUz3B0uZOA3QfNzSmsTz+eT86Rdg3TNQ@mail.gmail.com>

Hi!Thanks for the feedback.

On 10.04.2020 20:02, Yaroslav Dynnikov wrote:
> Hi, Leonid.
> 
> The first patch ("Add the chdir option for make" 8e8db7f 
> <https://github.com/tarantool/luarocks/commit/8e8db7f255a855c585245c288414ceb097576e07>) 
> looks good to
> me. I hope you'll be able to find time and propose it to upstream, in my
> opinion it'll helpful there too.
> 
> As of the second one ("Delete flags which can't be used with
> tarantoolctl rocks" 1b51b2f 
> <https://github.com/tarantool/luarocks/commit/1b51b2fe1c194719a7a8a74220a954a91c21f2a7>), 
> I'm worried that our fork every day goes
> farther from upstream. You didn't say a word about necessity to /remove/
> it, but it looks like another postponed problem. Luarocks version is
> already 3.3.1, while we still basing 3.1.1. And every such commit makes
> it harder to rebase. >
> The last time we were upgrading luarocks we've spent several weeks of
> worktime to resolve all the problems. And, disappointingly, those
> changes weren't merged to 1.10, so my colleagues usually come back with
> the same problems as 2 years ago. Please, don't screw over our future
> selves.
In my view, argument parsing is now implemented conceptually incorrectly 
because the LuaRoks flags are parsed first according to the tarantoolctl 
rules and then according to the LuaRocks rules, and these parsings have 
a mutual influence. When upgrading to a new version of LuaRocks (in case 
of changing the list of supported flags), you will must to analyze which 
flags to support, if the flags have not changed, there will be no 
conflicts. If you have two parsers, you will need to constantly maintain 
them in a consistent state and flags other commands will be affect to 
LuaRocks. So, in my horrible opinion if you have a conflict after rebase
here, you must resolve it at the rebase time.
> 
> Speaking about the patch in tarantool itself ("rocks: forward options to
> luarocks" 28a3b55f 
> <https://github.com/tarantool/tarantool/commit/28a3b55fbc94c10b284201955e8030de75e03f3f>), 
> I've run some commands and it seems to be OK.
> But I agree with Vlad that documentation should be more detailed. You
> can't just link the luarocks wiki (at first, because our version
> differs, but, moreover, because it's imperfect too). I don't believe
> anybody but you would be able to tell supported features from
> unsupported and find mistakes in wiki, so I suggest to clarify it
> somewhere (in the commit message or in any other place you like).
> 
I have no experience using LuaRocks. If you have an approximate list of 
differences that we want to add to documentation, I can apply it to 
default. But after all, commands and flags were added that we did not 
use, why their behavior is different from default?
> On Fri, 10 Apr 2020 at 03:00, Vladislav Shpilevoy 
> <v.shpilevoy@tarantool.org <mailto:v.shpilevoy@tarantool.org>> wrote:
> 
>     Hi!
> 
>     Thanks for the patchset!
> 
>     LGTM.
> 
>     But please, next time don't silently respond with a new patch
>     on my comments. I may be wrong sometimes. Honestly I am wrong
>     quite often. Like Vladimir said once, "mailing list exists for
>     discussions".
> 
>     I added Yaroslav to CC since he expressed a will to join the
>     review.
> 
>     On 09/04/2020 08:33, Leonid Vasiliev wrote:
>      > https://github.com/tarantool/tarantool/issues/4629
>      >
>     https://github.com/tarantool/luarocks/tree/lvasiliev/gh-4629-add-chdir-to-make
>      > See corresponding patch for tarantoolctl
>      >
>     (https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4629-forward-flags)
>      >
>      > Motivation:
>      > Imperfect integretion of the Luarocks to tarantoolctl
>      > (malfunctioning flags, disabled commands)
>      >
>      > Was done:
>      > whitelist of tarantoolctl don't used for luarocks flags
>      > Option chdir has been moved from tarantoolctl to luarocks
>      >
>      > @ChangeLog - see a comment in tarantool
>      >
>      > Leonid Vasiliev (2):
>      >   Add the chdir option for make
>      >   Delete flags which can't be used with tarantoolctl rocks
>      >
>      >  src/luarocks/cmd/help.lua | 5 -----
>      >  src/luarocks/cmd/make.lua | 8 ++++++++
>      >  src/luarocks/util.lua     | 5 +----
>      >  3 files changed, 9 insertions(+), 9 deletions(-)
>      >
> 
> 
> 
> -- 
> С уважением.
> Дынников Ярослав.

  reply	other threads:[~2020-04-10 18:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-09  6:33 Leonid Vasiliev
2020-04-09  6:33 ` [Tarantool-patches] [PATCH v3 1/2] Add the chdir option for make Leonid Vasiliev
2020-04-09  6:33 ` [Tarantool-patches] [PATCH v3 2/2] Delete flags which can't be used with tarantoolctl rocks Leonid Vasiliev
2020-04-13 11:59   ` lvasiliev
2020-04-13 12:16     ` Yaroslav Dynnikov
2020-04-10  0:00 ` [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl Vladislav Shpilevoy
2020-04-10  8:41   ` lvasiliev
2020-04-10 17:02   ` Yaroslav Dynnikov
2020-04-10 18:37     ` lvasiliev [this message]
2020-04-11 14:23     ` Vladislav Shpilevoy
2020-04-12 17:25       ` lvasiliev
2020-04-12 18:30         ` Yaroslav Dynnikov
2020-04-12 19:52           ` Vladislav Shpilevoy
2020-04-13  7:44           ` lvasiliev
2020-04-13 11:07 ` lvasiliev

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=6b4973cb-7c9e-3f18-c55d-12e30821da2e@tarantool.org \
    --to=lvasiliev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=yaroslav.dynnikov@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl' \
    /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