You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2019/08/19 15:56:35 UTC
[kudu-CR] small cleanup: remove unnecessary semicolons (part 2)
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14095
Change subject: small cleanup: remove unnecessary semicolons (part 2)
......................................................................
small cleanup: remove unnecessary semicolons (part 2)
Removed trailing semicolon in various macros that are defined using the
`do { ... } while (false)` clause and updated call sites where the
trailing semicolon was missing.
The motivation for this change was finding numerous warnings by the
built-in static analysis tools while browsing through the code using
the newer version of QtCreator IDE.
This patch does not contain any functional changes.
Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
---
M src/kudu/client/client-test.cc
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/partial_row.cc
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log_anchor_registry.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/fs/error_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/tombstoned_voting-imc-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/thrift/client.h
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/util/debug/trace_event.h
M src/kudu/util/env_posix.cc
M src/kudu/util/file_cache-stress-test.cc
M src/kudu/util/scoped_cleanup.h
M src/kudu/util/status.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/test_macros.h
M src/kudu/util/trace.h
34 files changed, 111 insertions(+), 116 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/14095/1
--
To view, visit http://gerrit.cloudera.org:8080/14095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
Gerrit-Change-Number: 14095
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] small cleanup: remove unnecessary semicolons (part 2)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14095
to look at the new patch set (#2).
Change subject: small cleanup: remove unnecessary semicolons (part 2)
......................................................................
small cleanup: remove unnecessary semicolons (part 2)
Removed trailing semicolon from the definitions of various macros
that are defined using the `do { ... } while (false)` clause.
Updated all call sites where the trailing semicolon was missing.
Being a minor change, this patch might break compilation of client
applications using Kudu C++ client's headers of prior versions
if RETURN_NOT_OK() and other macros were used without trailing semicolon
at call sites. However, due to the prevalent usage patterns of the
RETURN_NOT_OK() macro and friends in the existing code base and examples,
we should not expect anybody to be affected by this small cleanup.
The motivation for this change was finding numerous warnings output
by the built-in static analysis tools while browsing through the code
using the newer version of QtCreator IDE.
This patch does not contain any functional changes.
Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
---
M src/kudu/client/client-test.cc
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/partial_row.cc
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log_anchor_registry.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/fs/error_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/tombstoned_voting-imc-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/thrift/client.h
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/util/env_posix.cc
M src/kudu/util/file_cache-stress-test.cc
M src/kudu/util/scoped_cleanup.h
M src/kudu/util/status.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/test_macros.h
M src/kudu/util/trace.h
33 files changed, 98 insertions(+), 105 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/14095/2
--
To view, visit http://gerrit.cloudera.org:8080/14095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
Gerrit-Change-Number: 14095
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] small cleanup: remove unnecessary semicolons (part 2)
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14095 )
Change subject: small cleanup: remove unnecessary semicolons (part 2)
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
Gerrit-Change-Number: 14095
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 20 Aug 2019 04:21:16 +0000
Gerrit-HasComments: No
[kudu-CR] small cleanup: remove unnecessary semicolons (part 2)
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14095 )
Change subject: small cleanup: remove unnecessary semicolons (part 2)
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14095/1/src/kudu/util/debug/trace_event.h
File src/kudu/util/debug/trace_event.h:
http://gerrit.cloudera.org:8080/#/c/14095/1/src/kudu/util/debug/trace_event.h@887
PS1, Line 887: #define INTERNAL_TRACE_EVENT_ADD_SCOPED(category_group, name, ...) \
I think this change caused the trace tests to fail, probably due to the new scope.
http://gerrit.cloudera.org:8080/#/c/14095/1/src/kudu/util/status.h
File src/kudu/util/status.h:
PS1:
The changes potentially break API compatibility for clients that have used the macros and omitted a necessary semi-colon.
I don't think we care, but it's worth mentioning in the commit message.
--
To view, visit http://gerrit.cloudera.org:8080/14095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
Gerrit-Change-Number: 14095
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 19 Aug 2019 18:59:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] small cleanup: remove unnecessary semicolons (part 2)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14095 )
Change subject: small cleanup: remove unnecessary semicolons (part 2)
......................................................................
small cleanup: remove unnecessary semicolons (part 2)
Removed trailing semicolon from the definitions of various macros
that are defined using the `do { ... } while (false)` clause.
Updated all call sites where the trailing semicolon was missing.
Being a minor change, this patch might break compilation of client
applications using Kudu C++ client's headers of prior versions
if RETURN_NOT_OK() and other macros were used without trailing semicolon
at call sites. However, due to the prevalent usage patterns of the
RETURN_NOT_OK() macro and friends in the existing code base and examples,
we should not expect anybody to be affected by this small cleanup.
The motivation for this change was finding numerous warnings output
by the built-in static analysis tools while browsing through the code
using the newer version of QtCreator IDE.
This patch does not contain any functional changes.
Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
Reviewed-on: http://gerrit.cloudera.org:8080/14095
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/client/client-test.cc
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/partial_row.cc
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log_anchor_registry.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/fs/error_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/tombstoned_voting-imc-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/thrift/client.h
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/tool_action_cluster.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/util/env_posix.cc
M src/kudu/util/file_cache-stress-test.cc
M src/kudu/util/scoped_cleanup.h
M src/kudu/util/status.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/test_macros.h
M src/kudu/util/trace.h
33 files changed, 98 insertions(+), 105 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
Gerrit-Change-Number: 14095
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] small cleanup: remove unnecessary semicolons (part 2)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14095 )
Change subject: small cleanup: remove unnecessary semicolons (part 2)
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14095/1/src/kudu/util/debug/trace_event.h
File src/kudu/util/debug/trace_event.h:
http://gerrit.cloudera.org:8080/#/c/14095/1/src/kudu/util/debug/trace_event.h@887
PS1, Line 887: #define INTERNAL_TRACE_EVENT_ADD_SCOPED(category_group, name, ...) \
> I think this change caused the trace tests to fail, probably due to the new
Whoops, indeed -- those are related to the scope, so no new scope should be introduced. Good catch!
http://gerrit.cloudera.org:8080/#/c/14095/1/src/kudu/util/status.h
File src/kudu/util/status.h:
PS1:
> The changes potentially break API compatibility for clients that have used
Yup, I think due to the prevalent usage patterns of RETURN_NOT_OK() and friends, we should not expect many are affected by this update.
I added a note into the commit message.
--
To view, visit http://gerrit.cloudera.org:8080/14095
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59cbffccdfe9ca0dcd634515d041213420675dec
Gerrit-Change-Number: 14095
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 20 Aug 2019 01:05:30 +0000
Gerrit-HasComments: Yes