[tarantool-patches] Re: [PATCH] sql: remove struct Table

["i.koptelov" <ivan.koptelov@tarantool.org>]

> On 12 Feb 2019, at 17:56, n.pettik <korablev@tarantool.org> wrote:
> 
> 
> 
>> On 12 Feb 2019, at 16:55, i.koptelov <ivan.koptelov@tarantool.org> wrote:
>>> On 11 Feb 2019, at 20:58, n.pettik <korablev@tarantool.org> wrote:
>>>> On 6 Feb 2019, at 20:17, i.koptelov <ivan.koptelov@tarantool.org> wrote:
>>>> 
>>>>>> On 1 Feb 2019, at 14:05, Ivan Koptelov<ivan.koptelov@tarantool.org>  wrote:
>>>>>> 
>>>>>> Thank you for the review! Sorry for all these code style errors.
>>>>>> I consider review changes minor, so I put diff b/w first and
>>>>>> second version of patch at the end of the letter.
>>>>>> (second commit on the branch 'review fixes' would be squashed)
>>>>> Don’t do it next time, pls. Instead, inline fixes as an answers to comments,
>>>>> especially when it comes for huge diff ( 209 insertions(+), 259 deletions(-))
>>>>> Otherwise, it takes a while to track fixed chunks of code in a whole diff.
>>>> Sorry for this. All review fixes below is inlined.
>>> 
>>> Well, thank you. But don’t put fixes in a separate commit, at least when
>>> you push your branch to the remote repository. You may *accidentally*
>>> fail during final commit squashing and no-one will notice that.
>>> 
>>> Travis status was entirely negative: It can’t be compiled. 
>>> You forgot to add forward declaration of space_def.
>>> I fixed that and a couple of minor issues more. Here is a diff
>>> (I’ve squashed your commits and pushed on top of your branch).
>>> 
>>> Please, check it out. If it is OK, squash them and then patch LGTM.
>> Thank you for the fixes! Ok, I would not put my fixes in separate commit in future.
>> I checked out the diff, it’s ok for me except one small issue. I’ve marked
>> it below.
>>> 
>>> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
>>> index 874fd682f..1352f6efe 100644
>>> --- a/src/box/sql/sqliteInt.h
>>> +++ b/src/box/sql/sqliteInt.h
>>> @@ -2759,6 +2760,9 @@ struct Parse {
>>>      TriggerPrg *pTriggerPrg;        /* Linked list of coded triggers */
>>>      With *pWith;            /* Current WITH clause, or NULL */
>>>      With *pWithToFree;      /* Free this WITH object at the end of the parse */
>>> +       /** Space triggers are being coded for. */
>>> +       struct space *triggered_space;
>>> +       /** A space being constructed by CREATE TABLE */
>> Are you sure that this comment (last line) should be here? Is it about all lines below?
> 
> I forgot to move struct space *new_space; below.
> Thanks for cross-check. Could you please fix this?
> 
Fixed and pushed on branch.