From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 15 Aug 2019 01:23:48 +0300 From: Konstantin Osipov Subject: Re: [tarantool-patches] [PATCH v2 4/8] lua: rework luaL_field types to support msgpack extensions Message-ID: <20190814222348.GF28533@atlas> References: <8732ff5bc9cfe6622a14e773a76053ee1a20fcf8.1565263272.git.sergepetrenko@tarantool.org> <20190812212329.GF32337@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: To: Serge Petrenko Cc: Vladimir Davydov , tarantool-patches@freelists.org List-ID: * Serge Petrenko [19/08/13 16:44]: > Sorry for the confusion. I guess mp_field_type is just a bad name. > It has nothing to do with enum field_type. It just represents all the > messagepack types, including the extension types. No it doesn't. Please read here what messagepack types are: https://en.wikipedia.org/wiki/MessagePack It stores messagepack format codes and extension types. This is confusing. > It helps to eliminate a switch for extension type every time we get an > MP_EXT. So, it only deals with mp_type and mp_user_type. I get that. > > Look at mp_type when you can, and when you get MP_EXT or is > > otherwise confused, look at field_type? > > That’s what I’ve done in previous revision, but Vladimir didn’t like it. > And having implemented both variants, I agree that this enum simplifies > the code quite a lot. This is especially noticeable with patch 6. > > So, my proposal is to change the name to something better. > value_type? mp_value_type maybe? Could you please show me the previous version of the patch? -- Konstantin Osipov, Moscow, Russia