[tarantool-patches] Re: [PATCH 3/3] Add error messages

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the fixes! Finally the errors have become pretty.

>> 4. Same as 2.
> Fixed along with
> "bucket moving to " -> "bucket moving to "

What the difference? "bucket moving to" == "bucket moving to".

>>
>>>       end
>>>       assert(not errcode)
>>>       assert(mode == 'read' and bucket_is_readable(bucket) or
>>>              mode == 'write' and bucket_is_writable(bucket))
>>> -::finish::
>>> +::wrong_bucket::
>>
>> 6. The code below is executed both on error and on success. But
>> label is named 'wrong_bucket'. It is not correct. On success
>> path it is guaranteed that bucket ~= nil.
> Renamed back to `finish`. IMHO `wrong_bucket` suited well here.

You had labeled the success code as error processing. It is not ok.
And in the same code you check 'if bucket and ...', but bucket on
success is not nil - it is guaranteed. We must optimize the common
case. And the bucket is ok most of time.

+-- Only WRONG_BUCKET errors use this label.

This is wrong. See three lines above - they process a case when the
bucket is ok. You can label it ::wrong_bucket::, but then you must
separate this code from success path. I refactored this function,
you can look at it on the master branch.

The patchset is pushed with my fixes.