<div dir="ltr"><div>Hi, Leonid.</div><div><br></div><div>The first patch ("Add the chdir option for make" <a href="https://github.com/tarantool/luarocks/commit/8e8db7f255a855c585245c288414ceb097576e07">8e8db7f</a>) looks good to</div><div>me. I hope you'll be able to find time and propose it to upstream, in my</div><div>opinion it'll helpful there too.</div><div><br></div><div>As of the second one ("Delete flags which can't be used with</div><div>tarantoolctl rocks" <a href="https://github.com/tarantool/luarocks/commit/1b51b2fe1c194719a7a8a74220a954a91c21f2a7">1b51b2f</a>), I'm worried that our fork every day goes</div><div>farther from upstream. You didn't say a word about necessity to <i>remove</i></div><div>it, but it looks like another postponed problem. Luarocks version is<br>already 3.3.1, while we still basing 3.1.1. And every such commit makes<br>it harder to rebase.<br><br>The last time we were upgrading luarocks we've spent several weeks of<br>worktime to resolve all the problems. And, disappointingly, those<br>changes weren't merged to 1.10, so my colleagues usually come back with<br>the same problems as 2 years ago. Please, don't screw over our future</div><div>selves.</div><div><br></div><div>Speaking about the patch in tarantool itself ("rocks: forward options to</div><div>luarocks" <a href="https://github.com/tarantool/tarantool/commit/28a3b55fbc94c10b284201955e8030de75e03f3f">28a3b55f</a>), I've run some commands and it seems to be OK.<br></div><div>But I agree with Vlad that documentation should be more detailed. You<br>can't just link the luarocks wiki (at first, because our version<br>differs, but, moreover, because it's imperfect too). I don't believe<br>anybody but you would be able to tell supported features from<br>unsupported and find mistakes in wiki, so I suggest to clarify it<br>somewhere (in the commit message or in any other place you like).</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 10 Apr 2020 at 03:00, Vladislav Shpilevoy <<a href="mailto:v.shpilevoy@tarantool.org">v.shpilevoy@tarantool.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi!<br>
<br>
Thanks for the patchset!<br>
<br>
LGTM.<br>
<br>
But please, next time don't silently respond with a new patch<br>
on my comments. I may be wrong sometimes. Honestly I am wrong<br>
quite often. Like Vladimir said once, "mailing list exists for<br>
discussions".<br>
<br>
I added Yaroslav to CC since he expressed a will to join the<br>
review.<br>
<br>
On 09/04/2020 08:33, Leonid Vasiliev wrote:<br>
> <a href="https://github.com/tarantool/tarantool/issues/4629" rel="noreferrer" target="_blank">https://github.com/tarantool/tarantool/issues/4629</a><br>
> <a href="https://github.com/tarantool/luarocks/tree/lvasiliev/gh-4629-add-chdir-to-make" rel="noreferrer" target="_blank">https://github.com/tarantool/luarocks/tree/lvasiliev/gh-4629-add-chdir-to-make</a><br>
> See corresponding patch for tarantoolctl<br>
> (<a href="https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4629-forward-flags" rel="noreferrer" target="_blank">https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4629-forward-flags</a>)<br>
> <br>
> Motivation:<br>
> Imperfect integretion of the Luarocks to tarantoolctl<br>
> (malfunctioning flags, disabled commands)<br>
> <br>
> Was done:<br>
> whitelist of tarantoolctl don't used for luarocks flags<br>
> Option chdir has been moved from tarantoolctl to luarocks<br>
> <br>
> @ChangeLog - see a comment in tarantool<br>
> <br>
> Leonid Vasiliev (2):<br>
>   Add the chdir option for make<br>
>   Delete flags which can't be used with tarantoolctl rocks<br>
> <br>
>  src/luarocks/cmd/help.lua | 5 -----<br>
>  src/luarocks/cmd/make.lua | 8 ++++++++<br>
>  src/luarocks/util.lua     | 5 +----<br>
>  3 files changed, 9 insertions(+), 9 deletions(-)<br>
> <br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><span><div><div dir="ltr">С уважением. <br>Дынников Ярослав.<br></div></div></span></div></div>