Re: [Tarantool-patches] [PATCH 1/3] merger: drop luaL prefix where contract allows it

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

Thanks for the patch!

On 01/06/2020 20:10, Alexander Turenko wrote:
> This change highlights the contract of merge source virtual methods:
> they don't require a Lua state to be passed with arguments. Internal use
> of a temporary Lua state is the implementation detail. Technically the
> functions lean on a Lua state existence in a fiber storage, but the next
> commit will relax this requirement.
> 
> Made the following renames:
> 
> * luaL_merge_source_buffer_fetch   -> merge_source_buffer_fetch
> * luaL_merge_source_buffer_next    -> merge_source_buffer_next
> * luaL_merge_source_buffer_destroy -> merge_source_buffer_destroy
> 
> * keep luaL_merge_source_table_fetch (pass <struct lua_State *>)
> * luaL_merge_source_table_next     -> merge_source_table_next
> * luaL_merge_source_table_destroy  -> merge_source_table_destroy
> 
> * keep luaL_merge_source_tuple_fetch (change arguments order)
> * luaL_merge_source_tuple_next     -> merge_source_tuple_next
> * luaL_merge_source_tuple_destroy  -> merge_source_tuple_destroy
> 
> Also added API comments for destroy() and next() virtual methods to
> uniform them visually with other merge source functions.

I don't get why do you need these renames. merge_source API is
located in box/merger.h and box/merger.c. In lua/merger you have
children of struct merge_source. So they are not merge_source. The
latter is a virtual struct. lua/merger merge_source structs are
implementations of this virtual struct. So better not to use the same
prefix as for the top level merge_source API. IMO.