From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 13 Aug 2019 00:34:54 +0300 From: Konstantin Osipov Subject: Re: [PATCH v2 6/8] decimal: allow to encode/decode decimals as MsgPack Message-ID: <20190812213454.GI32337@atlas> References: <820cab72f09d48cf2d9e9cba5e0bba9620e32d96.1565263272.git.sergepetrenko@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <820cab72f09d48cf2d9e9cba5e0bba9620e32d96.1565263272.git.sergepetrenko@tarantool.org> To: Serge Petrenko Cc: vdavydov.dev@gmail.com, tarantool-patches@freelists.org List-ID: * Serge Petrenko [19/08/08 15:01]: > index d84748b71..89ea26b41 100644 > --- a/src/lib/core/mp_user_types.h > +++ b/src/lib/core/mp_user_types.h > @@ -31,6 +31,12 @@ > * SUCH DAMAGE. > */ > > +#include "msgpuck.h" why this include? What is this header file standing (mp_user_tpyes.h) for? User types are enum field_type. What other user types do you have in mind? Perhaps you mean user-defined types? Well, decimal is not a user defined type, it's built-in. The fact that it has to use mp_ext format type in msgpack is just a coincidence of Tarantool using msgpack for its internal data representation. > +++ b/src/lib/core/mpstream.h > @@ -32,6 +32,7 @@ > */ > > #include "diag.h" > +#include "decimal.h" > why this include? Please, please, C is not PHP, you don't include everything around just because it was a bad day. > index 6fc2b8278..2e6b5c163 100644 > --- a/src/lua/msgpack.c > +++ b/src/lua/msgpack.c > @@ -41,6 +41,11 @@ > #include > #include > > +#include "lua/decimal.h" > +#include "lib/core/decimal.h" > +#include "lib/core/mp_decimal.h" > +#include "lib/core/mp_user_types.h" > + This is a dependency hell. Why did you add a separate header for decimal encoding/encoding if you have to include *everything* in here anyway? Noone will ever be able to understand why you decided to include all of these here. -- Konstantin Osipov, Moscow, Russia