From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106]) (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 30E924696C3 for ; Fri, 10 Apr 2020 20:02:14 +0300 (MSK) Received: by smtp46.i.mail.ru with esmtpa (envelope-from ) id 1jMx2j-0003lo-Kk for tarantool-patches@dev.tarantool.org; Fri, 10 Apr 2020 20:02:13 +0300 Received: by mail-lj1-f182.google.com with SMTP id z26so2548717ljz.11 for ; Fri, 10 Apr 2020 10:02:13 -0700 (PDT) MIME-Version: 1.0 References: <865fb768-3c90-ec8e-96cf-7d7598a9f6e0@tarantool.org> In-Reply-To: <865fb768-3c90-ec8e-96cf-7d7598a9f6e0@tarantool.org> From: Yaroslav Dynnikov Date: Fri, 10 Apr 2020 20:02:01 +0300 Message-ID: Content-Type: multipart/alternative; boundary="000000000000dc559605a2f2b161" Subject: Re: [Tarantool-patches] [PATCH v3 0/2] Adapt luarocks for tarantoolctl List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: Yaroslav Dynnikov , tarantool-patches@dev.tarantool.org --000000000000dc559605a2f2b161 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, Leonid. The first patch ("Add the chdir option for make" 8e8db7f ) 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 ), 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. Speaking about the patch in tarantool itself ("rocks: forward options to luarocks" 28a3b55f ), 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). On Fri, 10 Apr 2020 at 03:00, Vladislav Shpilevoy 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-fla= gs > ) > > > > 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(-) > > > --=20 =D0=A1 =D1=83=D0=B2=D0=B0=D0=B6=D0=B5=D0=BD=D0=B8=D0=B5=D0=BC. =D0=94=D1=8B=D0=BD=D0=BD=D0=B8=D0=BA=D0=BE=D0=B2 =D0=AF=D1=80=D0=BE=D1=81= =D0=BB=D0=B0=D0=B2. --000000000000dc559605a2f2b161 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi, Leonid.

The first patch = ("Add the chdir option for make" 8e8db7f) looks good to

tarantoolctl rocks" 1b51b2f), I'm worried that our fork every day goes
f= arther from upstream. You didn't say a word about necessity to remov= e
it, but it looks like another postponed problem. Luarocks v= ersion is
already 3.3.1, while we still basing 3.1.1. And every such com= mit makes
it harder to rebase.

The last time we were upgrading lu= arocks we've spent several weeks of
worktime to resolve all the prob= lems. 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.

Speaking about the patch in tarantool itself ("rocks: forward = options to
luarocks" 28a3b55f), = 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 jus= t link the luarocks wiki (at first, because our version
differs, but, mo= reover, 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 me= ssage or in any other place you like).

On Fri, 10 Apr 2020 at 03:00, V= ladislav Shpilevoy <v.shpil= evoy@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.co= m/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):
>=C2=A0 =C2=A0Add the chdir option for make
>=C2=A0 =C2=A0Delete flags which can't be used with tarantoolctl roc= ks
>
>=C2=A0 src/luarocks/cmd/help.lua | 5 -----
>=C2=A0 src/luarocks/cmd/make.lua | 8 ++++++++
>=C2=A0 src/luarocks/util.lua=C2=A0 =C2=A0 =C2=A0| 5 +----
>=C2=A0 3 files changed, 9 insertions(+), 9 deletions(-)
>


--
=D0=A1 =D1=83= =D0=B2=D0=B0=D0=B6=D0=B5=D0=BD=D0=B8=D0=B5=D0=BC.=C2=A0
=D0=94=D1=8B=D0= =BD=D0=BD=D0=B8=D0=BA=D0=BE=D0=B2 =D0=AF=D1=80=D0=BE=D1=81=D0=BB=D0=B0=D0= =B2.
--000000000000dc559605a2f2b161--