[tarantool-patches] Re: [PATCH 2/2] sql: remove GLOB from Tarantool

[Alexander Turenko]

On Wed, Nov 14, 2018 at 05:16:52PM +0300, n.pettik wrote:
> 
> 
> > On 13 Nov 2018, at 22:23, Alexander Turenko <alexander.turenko at tarantool.org> wrote:
> > 
> > Hi!
> > 
> > I gave some minor comments and I think I cannot add any value anymore.
> > Nikita N., can you look into this patch?
> 
> >    Part of #3589
> >    Part of #3572
> >    Needed for #3251
> >    Needed for #3334
> 
> Why this patch is ’needed for’ but comes after
> these issues are closed by previous patch?
> 
> Also I don’t like the fact that your patch not only removes
> GLOB function, but also changes some user-visible interface.
> It still would be OK if you wrote about it in commit message
> or in doc request. For instance, as I understand from code,
> percent symbol (‘%’) now works as star (‘*’) did before.
> 

'*' from GLOB is subject to remove, '%' from LIKE still doing its work.
Comments that use names of symbols from GLOB are changed to use LIKE
syntax.

> Personally I don’t like many code style fixes because
> they obfuscate patch and make it complicate to review.
> What is more, they don’t deal with all problems: they
> partially fix comments but don’t fix naming. It looks awful.
> But since it is done and two reviewers already gave their
> OK I guess it would be irrational to start from scratch.
> 

Agreed. We should push developers to do refactoring in a separate commit
underneath actual changes.

> Finally, to be honest I didn’t review ALL changed tests.
> I looked through and they seem to be OK.
> 
> I’ve pushed also several non-functional fixes -
> see branch np/gh-3251-where-like-hangs.
> 

Don't have objections against your changes.

> Let's assume this as formal LGTM.