[tarantool-patches] Re: [PATCH v2 3/5] Merge box_error, stat and collations into core library

[Vladislav Shpilevoy]

Hello. Thanks for review!

On 04/05/2018 14:36, Alexander Turenko wrote:
> Vlad,
> 
> One comment is below.
> 
> WBR, Alexander Turenko.
> 
> On Sun, Apr 29, 2018 at 01:45:11AM +0300, Vladislav Shpilevoy wrote:
>> The goal is to expose collations into Lua with no dependencies on
>> box library. But collations merge into core requires box_error
>> and stat libraries too.
> 
>> diff --git a/src/box/lua/call.c b/src/box/lua/call.c
>> index be13812aa..b60c6c397 100644
>> --- a/src/box/lua/call.c
>> +++ b/src/box/lua/call.c
>> @@ -30,7 +30,7 @@
>>    */
>>   #include "box/lua/call.h"
>>   #include "box/call.h"
>> -#include "box/error.h"
>> +#include <error.h>
>>   #include "fiber.h"
>>   
>>   #include "lua/utils.h"
>> diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
>> index 314907421..960ea2aa9 100644
>> --- a/src/box/lua/error.cc
>> +++ b/src/box/lua/error.cc
>> @@ -40,7 +40,7 @@ extern "C" {
>>   #include <errinj.h>
>>   
>>   #include "lua/utils.h"
>> -#include "box/error.h"
>> +#include "src/error.h"
>>   
>>   static int
>>   luaT_error_raise(lua_State *L)
> 
> Don't know what policy is in use in Tarantool. The diff above shows two
> different approaches to include the same header within the one commit.
> 
> It seems that we don't use relative includes (with ..) and prefer either
> <> or "src/..." w/o any particular system. Right?

Right. We do not use relative ways. But "src/error.h" looks ugly, I replaced it
with <error.h>:
diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
index 960ea2aa9..bf9e5bac4 100644
--- a/src/box/lua/error.cc
+++ b/src/box/lua/error.cc
@@ -38,9 +38,9 @@ extern "C" {
  
  #include <fiber.h>
  #include <errinj.h>
+#include <error.h>
  
  #include "lua/utils.h"
-#include "src/error.h"
  
  static int
  luaT_error_raise(lua_State *L)

For more examples about <> and "" difference you can look at the same file src/box/lua/error.cc:

#include <fiber.h>
#include <errinj.h>

these files are in <> in lua/ dir, but in "" in box/ dir.