[PATCH v2 6/8] decimal: allow to encode/decode decimals as MsgPack

Konstantin Osipov kostja at tarantool.org
Tue Aug 13 00:34:54 MSK 2019


* Serge Petrenko <sergepetrenko at tarantool.org> [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 <small/region.h>
>  #include <small/ibuf.h>
>  
> +#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



More information about the Tarantool-patches mailing list