From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 00621276C6 for ; Tue, 12 Feb 2019 08:55:18 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FMN2DgsXS6ta for ; Tue, 12 Feb 2019 08:55:17 -0500 (EST) Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 4C4C924980 for ; Tue, 12 Feb 2019 08:55:17 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH] sql: remove struct Table From: "i.koptelov" In-Reply-To: <7CB3BC6E-B4CF-4EE0-BFB7-F5E7CFF89947@tarantool.org> Date: Tue, 12 Feb 2019 16:55:14 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <4c2b87b5-9e83-f96f-4350-edf7a12e5593@tarantool.org> <9f612f00-6222-e3cb-642c-04bfec116fd4@tarantool.org> <3438F900-EF6D-4475-B3F1-BD4971F78186@tarantool.org> <7CB3BC6E-B4CF-4EE0-BFB7-F5E7CFF89947@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: "n.pettik" > On 11 Feb 2019, at 20:58, n.pettik wrote: >=20 >=20 >> On 6 Feb 2019, at 20:17, i.koptelov = wrote: >>=20 >>>> On 1 Feb 2019, at 14:05, Ivan Koptelov = wrote: >>>>=20 >>>> 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=E2=80=99t 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. >=20 > Well, thank you. But don=E2=80=99t 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. >=20 > Travis status was entirely negative: It can=E2=80=99t be compiled.=20 > You forgot to add forward declaration of space_def. > I fixed that and a couple of minor issues more. Here is a diff > (I=E2=80=99ve squashed your commits and pushed on top of your branch). >=20 > 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=E2=80=99s ok for me except one small issue. = I=E2=80=99ve marked it below. >=20 > 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? > /** > * Number of FK constraints declared within > * CREATE TABLE statement.