[tarantool-patches] Re: [PATCH v2 1/1] netbox: show is_nullable and collation fields

[tarantool-patches] Re: [PATCH v2 1/1] netbox: show is_nullable and collation fields

[v.shpilevoy]

See below 2 minor comments.

> 26 марта 2018 г., в 20:20, Kirill Shcherbatov <kshcherbatov@tarantool.org> написал(а):
> 
> Fixes #3256
> ---
> 
> Issue: https://github.com/tarantool/tarantool/issues/3256
> Branch: https://github.com/tarantool/tarantool/compare/gh-3256-net-box-nullable-collation-options-output
> 
> src/box/lua/net_box.lua   |  6 ++++-
> test/box/net.box.result   | 65 +++++++++++++++++++++++++++++++++++++++++++++++
> test/box/net.box.test.lua | 24 +++++++++++++++++
> 3 files changed, 94 insertions(+), 1 deletion(-)
> 
> diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
> index c7541ed..fe45721 100644
> --- a/test/box/net.box.test.lua
> +++ b/test/box/net.box.test.lua
> @@ -798,3 +798,27 @@ c.space.test.index.test_index ~= nil
> box.schema.user.revoke('guest','read,write,execute','universe')
> 
> space:drop()
> +
> +

1. Please, do not put multiple empty lines, when it is not necessary.

> +--
> +-- gh-3256 net.box is_nullable and collation options output
> +--
> +
> +space = box.schema.create_space('test')
> +box.schema.user.grant('guest','read,write,execute','space', 'test')
> +pk = space:create_index('pk')
> +sk = space:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}})
> +c = net:connect(box.cfg.listen)
> +c.space.test.index.sk.parts
> +c:close()
> +space:drop()
> +
> +space = box.schema.create_space('test')
> +box.schema.user.grant('guest','read,write,execute','space', 'test')
> +box.internal.collation.create('test', 'ICU', 'ru-RU')
> +sk = space:create_index('sk', { type = 'tree', parts = {{1, 'str', collation = 'test'}}, unique = true })
> +c = net:connect(box.cfg.listen)
> +c.space.test.index.sk.parts
> +c:close()
> +box.internal.collation.drop('test')
> +space:drop()
> \ No newline at end of file

2. Please, add an empty line at the end of the file.

> -- 
> 2.7.4
> 

[tarantool-patches] Re: [PATCH v2 1/1] netbox: show is_nullable and collation fields

[Kirill Shcherbatov]

diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index fe45721..3b4ce05 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -799,11 +799,9 @@ 
box.schema.user.revoke('guest','read,write,execute','universe')

  space:drop()

-
  --
  -- gh-3256 net.box is_nullable and collation options output
  --
-
  space = box.schema.create_space('test')
  box.schema.user.grant('guest','read,write,execute','space', 'test')
  pk = space:create_index('pk')
@@ -821,4 +819,5 @@ c = net:connect(box.cfg.listen)
  c.space.test.index.sk.parts
  c:close()
  box.internal.collation.drop('test')
-space:drop()
\ No newline at end of file
+space:drop()
+


On 26.03.2018 20:24, v.shpilevoy@tarantool.org wrote:
> See below 2 minor comments.
>
>> 26 марта 2018 г., в 20:20, Kirill Shcherbatov <kshcherbatov@tarantool.org> написал(а):
>>
>> Fixes #3256
>> ---
>>
>> Issue: https://github.com/tarantool/tarantool/issues/3256
>> Branch: https://github.com/tarantool/tarantool/compare/gh-3256-net-box-nullable-collation-options-output
>>
>> src/box/lua/net_box.lua   |  6 ++++-
>> test/box/net.box.result   | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>> test/box/net.box.test.lua | 24 +++++++++++++++++
>> 3 files changed, 94 insertions(+), 1 deletion(-)
>>
>> diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
>> index c7541ed..fe45721 100644
>> --- a/test/box/net.box.test.lua
>> +++ b/test/box/net.box.test.lua
>> @@ -798,3 +798,27 @@ c.space.test.index.test_index ~= nil
>> box.schema.user.revoke('guest','read,write,execute','universe')
>>
>> space:drop()
>> +
>> +
> 1. Please, do not put multiple empty lines, when it is not necessary.
>
>> +--
>> +-- gh-3256 net.box is_nullable and collation options output
>> +--
>> +
>> +space = box.schema.create_space('test')
>> +box.schema.user.grant('guest','read,write,execute','space', 'test')
>> +pk = space:create_index('pk')
>> +sk = space:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}})
>> +c = net:connect(box.cfg.listen)
>> +c.space.test.index.sk.parts
>> +c:close()
>> +space:drop()
>> +
>> +space = box.schema.create_space('test')
>> +box.schema.user.grant('guest','read,write,execute','space', 'test')
>> +box.internal.collation.create('test', 'ICU', 'ru-RU')
>> +sk = space:create_index('sk', { type = 'tree', parts = {{1, 'str', collation = 'test'}}, unique = true })
>> +c = net:connect(box.cfg.listen)
>> +c.space.test.index.sk.parts
>> +c:close()
>> +box.internal.collation.drop('test')
>> +space:drop()
>> \ No newline at end of file
> 2. Please, add an empty line at the end of the file.
>
>> -- 
>> 2.7.4
>>
>

Re: [tarantool-patches] Re: [PATCH v2 1/1] netbox: show is_nullable and collation fields

[v.shpilevoy]

Looks good to me. Vova, please, take a look.

> 26 марта 2018 г., в 20:41, Kirill Shcherbatov <kshcherbatov@tarantool.org> написал(а):
> 
> diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
> index fe45721..3b4ce05 100644
> --- a/test/box/net.box.test.lua
> +++ b/test/box/net.box.test.lua
> @@ -799,11 +799,9 @@ box.schema.user.revoke('guest','read,write,execute','universe')
> 
>  space:drop()
> 
> -
>  --
>  -- gh-3256 net.box is_nullable and collation options output
>  --
> -
>  space = box.schema.create_space('test')
>  box.schema.user.grant('guest','read,write,execute','space', 'test')
>  pk = space:create_index('pk')
> @@ -821,4 +819,5 @@ c = net:connect(box.cfg.listen)
>  c.space.test.index.sk.parts
>  c:close()
>  box.internal.collation.drop('test')
> -space:drop()
> \ No newline at end of file
> +space:drop()
> +
> 
> 
> On 26.03.2018 20:24, v.shpilevoy@tarantool.org wrote:
>> See below 2 minor comments.
>> 
>>> 26 марта 2018 г., в 20:20, Kirill Shcherbatov <kshcherbatov@tarantool.org> написал(а):
>>> 
>>> Fixes #3256
>>> ---
>>> 
>>> Issue: https://github.com/tarantool/tarantool/issues/3256
>>> Branch: https://github.com/tarantool/tarantool/compare/gh-3256-net-box-nullable-collation-options-output
>>> 
>>> src/box/lua/net_box.lua   |  6 ++++-
>>> test/box/net.box.result   | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>>> test/box/net.box.test.lua | 24 +++++++++++++++++
>>> 3 files changed, 94 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
>>> index c7541ed..fe45721 100644
>>> --- a/test/box/net.box.test.lua
>>> +++ b/test/box/net.box.test.lua
>>> @@ -798,3 +798,27 @@ c.space.test.index.test_index ~= nil
>>> box.schema.user.revoke('guest','read,write,execute','universe')
>>> 
>>> space:drop()
>>> +
>>> +
>> 1. Please, do not put multiple empty lines, when it is not necessary.
>> 
>>> +--
>>> +-- gh-3256 net.box is_nullable and collation options output
>>> +--
>>> +
>>> +space = box.schema.create_space('test')
>>> +box.schema.user.grant('guest','read,write,execute','space', 'test')
>>> +pk = space:create_index('pk')
>>> +sk = space:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}})
>>> +c = net:connect(box.cfg.listen)
>>> +c.space.test.index.sk.parts
>>> +c:close()
>>> +space:drop()
>>> +
>>> +space = box.schema.create_space('test')
>>> +box.schema.user.grant('guest','read,write,execute','space', 'test')
>>> +box.internal.collation.create('test', 'ICU', 'ru-RU')
>>> +sk = space:create_index('sk', { type = 'tree', parts = {{1, 'str', collation = 'test'}}, unique = true })
>>> +c = net:connect(box.cfg.listen)
>>> +c.space.test.index.sk.parts
>>> +c:close()
>>> +box.internal.collation.drop('test')
>>> +space:drop()
>>> \ No newline at end of file
>> 2. Please, add an empty line at the end of the file.
>> 
>>> -- 
>>> 2.7.4
>>> 
>> 
> 
> 

Re: [tarantool-patches] Re: [PATCH v2 1/1] netbox: show is_nullable and collation fields

[Vladimir Davydov]

On Mon, Mar 26, 2018 at 08:41:21PM +0300, Kirill Shcherbatov wrote:
> diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
> index fe45721..3b4ce05 100644
> --- a/test/box/net.box.test.lua
> +++ b/test/box/net.box.test.lua
> @@ -799,11 +799,9 @@
> box.schema.user.revoke('guest','read,write,execute','universe')
> 
>  space:drop()
> 
> -
>  --
>  -- gh-3256 net.box is_nullable and collation options output
>  --
> -
>  space = box.schema.create_space('test')
>  box.schema.user.grant('guest','read,write,execute','space', 'test')
>  pk = space:create_index('pk')
> @@ -821,4 +819,5 @@ c = net:connect(box.cfg.listen)
>  c.space.test.index.sk.parts
>  c:close()
>  box.internal.collation.drop('test')
> -space:drop()
> \ No newline at end of file
> +space:drop()
> +

Nit-pick: extra newline at the end of the file. We always add exactly
one newline symbol at the end of each source file. In v1 you added no
newline at all, in v2 you added two newline symbols. Please fix your
editor configuration.

Other than that, the patch is fine by me.

Re: [tarantool-patches] Re: [PATCH v2 1/1] netbox: show is_nullable and collation fields

[Kirill Shcherbatov]

Removed extra newline at the end of test file. The branch has been also 
rebased to the 1.9

diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 3b4ce05..07555f1 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -820,4 +820,3 @@ c.space.test.index.sk.parts
  c:close()
  box.internal.collation.drop('test')
  space:drop()
-

On 27.03.2018 19:26, Vladimir Davydov wrote:
> On Mon, Mar 26, 2018 at 08:41:21PM +0300, Kirill Shcherbatov wrote:
>> diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
>> index fe45721..3b4ce05 100644
>> --- a/test/box/net.box.test.lua
>> +++ b/test/box/net.box.test.lua
>> @@ -799,11 +799,9 @@
>> box.schema.user.revoke('guest','read,write,execute','universe')
>>
>>   space:drop()
>>
>> -
>>   --
>>   -- gh-3256 net.box is_nullable and collation options output
>>   --
>> -
>>   space = box.schema.create_space('test')
>>   box.schema.user.grant('guest','read,write,execute','space', 'test')
>>   pk = space:create_index('pk')
>> @@ -821,4 +819,5 @@ c = net:connect(box.cfg.listen)
>>   c.space.test.index.sk.parts
>>   c:close()
>>   box.internal.collation.drop('test')
>> -space:drop()
>> \ No newline at end of file
>> +space:drop()
>> +
> Nit-pick: extra newline at the end of the file. We always add exactly
> one newline symbol at the end of each source file. In v1 you added no
> newline at all, in v2 you added two newline symbols. Please fix your
> editor configuration.
>
> Other than that, the patch is fine by me.