[tarantool-patches] Re: [PATCH][vshard] Check self passed to object methods

[Alex Khatskevich]

Hi.

> 1. Please, put links after --- below the commit message. Not before and
> not above your own ---.
>
ok
>
> 2. Why can not you just iterate over replicaset metatable? Your way
> requires to update this test each time a new method is added.
I thought, that it can create a lot of pain in case someone adds method 
which do not take
self attribute.
Anyway, fixed adding this helper:
-----------------------
get_object_method_names = function(object)
     local res = {}
     for name, _ in pairs(getmetatable(object).__index) do
         table.insert(res, name)
     end
     table.sort(res)
     return res
end;
-------------------------
>> +        connect_replica = function(rs, r)
>> +            assert(type(r) == "table",
>> +                   'Use replicaset:connect_replica(...) instead of ' ..
>> +                   'replicaset.connect_replica(...)')> +            
>> return replicaset_connect_to_replica(rs, r)
>
> 4. You have no checked rs. This check_self_arg is broken - you must 
> have two functions:
> check and generate checker. Now you have only generate checker that 
> you have named
> check_self_arg.
>
> In this function you do manual check_self_arg for 'r' and 'rs'. For other
> functions you do the same as now - generate checker.
Fixed this way:
             assert(type(rs) == "table" and type(r) == "table",
>
> 5. Describing a function you may omit 'this function does', 'this 
> function
> is needed to' etc. Function comment must look like a commit title:
> imperative text describing what this function shall do.
> 7. 'This function takes' must be moved to the corresponding @param.
Rewritten a little.

> 6. anguments?
> 8. modele?
> 9. You do not need extra new line after each @param.
Fixed

Here is a new version
--
-- Wrap a given function to check that self argument is passed.
-- New function returns an error in case one called a method
-- like object.func instead of object:func.
-- Returning wrapped function to a user and using raw functions inside of a
-- module improves speed.
-- This function can be used only in case the second argument is not of the
-- "table" type.
--
-- @param obj_name Name of the called object. Used only for error message
--        construction.
-- @param func_name Name of the called function. Used only for an error 
message
--        construction.
-- @param func A function which is about to be wrapped
--
-- @retval Wrapped function.
--

> 10. Why no just 'return function wrapped_func(... ' ?
Fixed