[tarantool-patches] Re: [PATCH] sql: remove struct Table
i.koptelov
ivan.koptelov at tarantool.org
Tue Feb 12 18:07:38 MSK 2019
> On 12 Feb 2019, at 17:56, n.pettik <korablev at tarantool.org> wrote:
>
>
>
>> On 12 Feb 2019, at 16:55, i.koptelov <ivan.koptelov at tarantool.org> wrote:
>>> On 11 Feb 2019, at 20:58, n.pettik <korablev at tarantool.org> wrote:
>>>> On 6 Feb 2019, at 20:17, i.koptelov <ivan.koptelov at tarantool.org> wrote:
>>>>
>>>>>> On 1 Feb 2019, at 14:05, Ivan Koptelov<ivan.koptelov at 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.
More information about the Tarantool-patches
mailing list