From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 18 Jul 2019 12:54:22 +0300 From: Konstantin Osipov Subject: Re: [tarantool-patches] [PATCH v3 4/4] box: introduce functional indexes in memtx Message-ID: <20190718095422.GE21191@atlas> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com, Kirill Shcherbatov List-ID: * Kirill Shcherbatov [19/07/17 09:39]: > +++ b/src/box/CMakeLists.txt > @@ -51,8 +51,9 @@ add_library(tuple STATIC > coll_id_cache.c > field_def.c > opt_def.c > + functional_extractor.c since we started to use func_ and not function_ and functional_ as an abbreviation, let's be consistent and use it everywere, so func_extractor. extractor is not a good name either (and most likely not a good division of responsibilities), but I'll get to it later. > + functional_key.c func_key when a patch adds new modules the commit comment should contain an explanation about what these modules do and why they were added. > index 1dbfe6b26..fdf38598e 100644 > --- a/src/box/alter.cc > +++ b/src/box/alter.cc > @@ -30,6 +30,7 @@ > */ > #include "alter.h" > #include "assoc.h" > +#include "box.h" Kirill, please stop bloating header file dependencies, especially circular ones. If you need to include a dependency like this, you haven't dealt with technical debt. There should be a separate patch which improves modules structure so that this dependency is avoided. The dependency issue is described in the SOP, so I should not have to point it out time and again. -- Konstantin Osipov, Moscow, Russia