From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 E3A84469719 for ; Sat, 26 Sep 2020 01:58:44 +0300 (MSK) References: From: Vladislav Shpilevoy Message-ID: <53487cd7-3903-99e1-eb77-aaf24c76f170@tarantool.org> Date: Sat, 26 Sep 2020 00:58:43 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 08/14] WIP: refactoring: extract key_def module API functions List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! See 2 comments below. On 23.09.2020 03:14, Alexander Turenko wrote: > I plan to expand module API for key_def in further commits and mix of > public and private structures and functions becomes hard to read. So it > looks meaningful to extract module API wrappers into its own compilation > unit. 1. I don't think it is a good idea. Not only it is inconsistent with other modules such as tuple and fiber, but also it makes not possible to use key_def.c static functions, and you need to start moving things to the header to be able to use them. In the result, the code becomes harder to follow. If you want to separate public and non-public methods, you may consider grouping them inside the same file. But moving to a new file does not look good. Also the new file name looks somewhat 'ugly'. You are basically trying to re-invent module.h and the way it is built but per-submodule. > Added libtuple.a to the so called reexport libraries list, because > otherwise the functions from key_def_api compilation unit are not > exported on 1.10 (in the backported patch). > > Part of #5273 > --- > diff --git a/src/box/key_def_api.h b/src/box/key_def_api.h > new file mode 100644 > index 000000000..5b1c861f5 > --- /dev/null > +++ b/src/box/key_def_api.h > @@ -0,0 +1,101 @@ > +#ifndef TARANTOOL_BOX_KEY_DEF_API_H_INCLUDED > +#define TARANTOOL_BOX_KEY_DEF_API_H_INCLUDED 2. Lets use '#pragma once'. > +/* > + * Copyright 2010-2020, Tarantool AUTHORS, please see AUTHORS file. > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * 1. Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the > + * following disclaimer. > + * > + * 2. Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED > + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL > + * OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, > + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */