From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109])
 (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 27EA3469719
 for <tarantool-patches@dev.tarantool.org>;
 Fri, 21 Feb 2020 01:44:36 +0300 (MSK)
References: <1582130122.751366742@f143.i.mail.ru>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <f926d12f-74fe-88ca-b86d-9a2e581ccb84@tarantool.org>
Date: Thu, 20 Feb 2020 23:44:33 +0100
MIME-Version: 1.0
In-Reply-To: <1582130122.751366742@f143.i.mail.ru>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [Tarantool-patches] [PATCH] box: user.grant error should be
 versatile
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Maria Khaydich <maria.khaydich@tarantool.org>, tarantool-patches <tarantool-patches@dev.tarantool.org>, Igor Munkin <imun@tarantool.org>

Hi! Thanks for the patch!

On 19/02/2020 17:35, Maria Khaydich wrote:
> Error message on granted privileges was not flexible and
> did not distinguish between universal or any other priviliges

priviliges -> privileges

> leaving either 'nil' or simply '' at the end.
>  
> Closes #714
> > diff --git a/test/box/access.result b/test/box/access.result
> index 9554991ad..be8b1c521 100644
> --- a/test/box/access.result
> +++ b/test/box/access.result
> @@ -532,7 +532,7 @@ box.space._priv:select{id}
>  ...
>  box.schema.user.grant('user', 'read', 'universe')
>  ---
> -- error: User 'user' already has read access on universe ''
> +- error: User 'user' already has read access on universe
>  ...
>  box.space._priv:select{id}
>  ---
> @@ -738,7 +738,7 @@ box.schema.user.grant('guest', 'read,write,execute', 'universe')
>  ...
>  box.schema.user.grant('guest', 'read,write,execute', 'universe')
>  ---
> -- error: User 'guest' already has read,write,execute access on universe ''
> +- error: User 'guest' already has read,write,execute access on universe
>  ...
>  box.schema.user.grant('guest', 'read,write,execute', 'universe', '', { if_not_exists = true })
>  ---
> @@ -2108,3 +2108,29 @@ box.space._priv:delete{1, 'universe', 0}
>  ---
>  - error: 'Incorrect grant arguments: can''t revoke universe from the admin user'
>  ...
> +--
> +-- gh-714: box.schema.user.grant error should be versatile,
> +-- i.e. error on universally granted privileges shouldn't
> +-- include any redundant details and/or symbols
> +--
> +box.schema.user.grant('guest', 'read,write,execute', 'universe')
> +---
> +...
> +box.schema.user.grant('guest', 'read,write,execute', 'universe')
> +---
> +- error: User 'guest' already has read,write,execute access on universe
> +...
> +-- expected behavior of grant() error shouldn't change otherwise

It would be good to have a dot in the end of this sentence and of the
previous comment. And to start the sentence with a capital letter.
Although I can't block the patch because of this nit, so LGTM and up
to you.