From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tsafin@tarantool.org>
Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id A9DAB445320
 for <tarantool-patches@dev.tarantool.org>;
 Tue, 14 Jul 2020 00:51:12 +0300 (MSK)
From: "Timur Safin" <tsafin@tarantool.org>
References: <1594199230-26036-1-git-send-email-alyapunov@tarantool.org>
 <1594199230-26036-2-git-send-email-alyapunov@tarantool.org>
In-Reply-To: <1594199230-26036-2-git-send-email-alyapunov@tarantool.org>
Date: Tue, 14 Jul 2020 00:51:11 +0300
Message-ID: <0e5801d6595f$b935e310$2ba1a930$@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: 7bit
Content-Language: ru
Subject: Re: [Tarantool-patches] [PATCH 1/2] alter: use good c++ style
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: 'Aleksandr Lyapunov' <alyapunov@tarantool.org>, tarantool-patches@dev.tarantool.org
Cc: v.shpilevoy@tarantool.org

: From: Aleksandr Lyapunov
: Subject: [Tarantool-patches] [PATCH 1/2] alter: use good c++ style
: 


: @@ -1392,14 +1388,14 @@ ModifyIndex::alter(struct alter_space *alter)
:  }
: 
:  void
: -ModifyIndex::commit(struct alter_space *alter, int64_t signature)
: +ModifyIndex::commit(struct alter_space *alter, int64_t signature)
: noexcept
:  {
:  	(void)alter;

This is C++, not C - we could simply omit argument name, and there is 
no need to dereference argument to make compiler shut

: @@ -1475,7 +1471,7 @@ CreateIndex::prepare(struct alter_space *alter)
:  }
: 
:  void
: -CreateIndex::commit(struct alter_space *alter, int64_t signature)
: +CreateIndex::commit(struct alter_space *alter, int64_t signature)
: noexcept
:  {
:  	(void) alter;

The same comment is applicable here - we could simply omit argument name


: @@ -1627,7 +1624,7 @@ TruncateIndex::prepare(struct alter_space *alter)
:  }
: 
:  void
: -TruncateIndex::commit(struct alter_space *alter, int64_t signature)
: +TruncateIndex::commit(struct alter_space *alter, int64_t signature)
: noexcept
:  {
:  	(void)alter;

And here... 

:  	index_commit_drop(old_index, signature);
: @@ -1652,14 +1649,14 @@ class UpdateSchemaVersion: public AlterSpaceOp
:  public:
:  	UpdateSchemaVersion(struct alter_space * alter)
:  		:AlterSpaceOp(alter) {}
: -	virtual void alter(struct alter_space *alter);
: +	void alter(struct alter_space *alter) noexcept override;
:  };
: 
:  void
: -UpdateSchemaVersion::alter(struct alter_space *alter)
: +UpdateSchemaVersion::alter(struct alter_space *alter) noexcept
:  {
: -    (void)alter;

And here ...

: -    ++schema_version;
: +	(void) alter;
: +	++schema_version;

But here we could ask _very important question_ (:)) do we use spaces
or tabs (like in C) for indenting C++ sources? Looks like Tabs are not yet
used in this file, thus no need to enforce the different style.


:  }
: 
:  /**
: @@ -1853,14 +1851,15 @@ CreateConstraintID::alter(struct alter_space
: *alter)
:  }
: 
:  void
: -CreateConstraintID::rollback(struct alter_space *alter)
: +CreateConstraintID::rollback(struct alter_space *alter) noexcept
:  {
:  	space_delete_constraint_id(alter->new_space, new_id->name);
:  	new_id = NULL;
:  }
: 
:  void
: -CreateConstraintID::commit(struct alter_space *alter, int64_t signature)
: +CreateConstraintID::commit(struct alter_space *alter,
: +                           int64_t signature) noexcept
:  {
:  	(void) alter;
:  	(void) signature;

Here we could omit names of both arguments ...

: -DropConstraintID::alter(struct alter_space *alter)
: +DropConstraintID::alter(struct alter_space *alter) noexcept
:  {
:  	old_id = space_pop_constraint_id(alter->old_space, name);
:  }
: 
:  void
: -DropConstraintID::commit(struct alter_space *alter, int64_t signature)
: +DropConstraintID::commit(struct alter_space *alter, int64_t signature)
: noexcept
:  {
:  	(void) alter;
:  	(void) signature;

The same comment is applicable here also...

Best Regards,
Timur