Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16916 )

Change subject: [rpc] update ServiceIf::service_name() signature
......................................................................


Patch Set 3:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:

http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@59
PS3, Line 59: using kudu::rpc_test::AddRequestPB;
> warning: using declarations in the global namespace in headers are prohibit
I'm not going to address this: this is test-only code and this header is used 
only in a few places.  Updating this code is out of the scope of the patch.


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@60
PS3, Line 60: using kudu::rpc_test::AddResponsePB;
> warning: using declarations in the global namespace in headers are prohibit
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@61
PS3, Line 61: using kudu::rpc_test::CalculatorError;
> warning: using declarations in the global namespace in headers are prohibit
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@83
PS3, Line 83: using kudu::rpc_test_diff_package::ReqDiffPackagePB;
> warning: using declarations in the global namespace in headers are prohibit
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@84
PS3, Line 84: using kudu::rpc_test_diff_package::RespDiffPackagePB;
> warning: using declarations in the global namespace in headers are prohibit
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@134
PS3, Line 134:   void DoAdd(InboundCall *incoming) {
> warning: method 'DoAdd' can be made static [readability-convert-member-func
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@402
PS3, Line 402: const std::string GenericCalculatorService::kFullServiceName = 
"kudu.rpc.GenericCalculatorService";
> warning: variable 'kFullServiceName' defined in a header file; variable def
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@403
PS3, Line 403: const char *GenericCalculatorService::kAddMethodName = "Add";
> warning: variable 'kAddMethodName' defined in a header file; variable defin
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@404
PS3, Line 404: const char *GenericCalculatorService::kSleepMethodName = "Sleep";
> warning: variable 'kSleepMethodName' defined in a header file; variable def
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/rpc-test-base.h@405
PS3, Line 405: const char 
*GenericCalculatorService::kSleepWithSidecarMethodName = "SleepWithSidecar";
> warning: variable 'kSleepWithSidecarMethodName' defined in a header file; v
ditto


http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.cc
File src/kudu/rpc/service_if.cc:

http://gerrit.cloudera.org:8080/#/c/16916/3/src/kudu/rpc/service_if.cc@149
PS3, Line 149: GeneratedServiceIf::GeneratedServiceIf(const 
scoped_refptr<ResultTracker>& tracker)
> warning: pass by value and use std::move [modernize-pass-by-value]
I'm not going to address this TidyBot comment since the signatures of the rest 
of involved methods do not pass this by value.



--
To view, visit http://gerrit.cloudera.org:8080/16916
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7fc425cf789fa31e99642061b6474b0effd489
Gerrit-Change-Number: 16916
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <[email protected]>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 04 Jan 2021 17:14:08 +0000
Gerrit-HasComments: Yes

Reply via email to