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

Change subject: rpc: replace gscoped_ptr with unique_ptr
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/15006/4/src/kudu/rpc/connection.cc
File src/kudu/rpc/connection.cc:

http://gerrit.cloudera.org:8080/#/c/15006/4/src/kudu/rpc/connection.cc@545
PS4, Line 545:                             Connection *conn) :
             :     call_(std::move(call)),
             :     conn_(conn)
nit: while you are here, maybe unify the indentation in the initializer list of 
affected classes?  E.g., here it would be

    : call_(std::move(call)),
      conn_(conn)

instead of current indentation.


http://gerrit.cloudera.org:8080/#/c/15006/4/src/kudu/rpc/mt-rpc-test.cc
File src/kudu/rpc/mt-rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/15006/4/src/kudu/rpc/mt-rpc-test.cc@217
PS4, Line 217: ()
nit: drop the parentheses?


http://gerrit.cloudera.org:8080/#/c/15006/4/src/kudu/rpc/outbound_call.cc
File src/kudu/rpc/outbound_call.cc:

http://gerrit.cloudera.org:8080/#/c/15006/4/src/kudu/rpc/outbound_call.cc@525
PS4, Line 525: transfer_.swap(transfer)
nit: would std::move() be any better here?


http://gerrit.cloudera.org:8080/#/c/15006/4/src/kudu/rpc/rpc_context.h
File src/kudu/rpc/rpc_context.h:

http://gerrit.cloudera.org:8080/#/c/15006/4/src/kudu/rpc/rpc_context.h@20
PS4, Line 20: stddef.h
nit: while you are here, replace with cstddef ?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I509e5cce6ae2c8351cea2fc68c7377d871ea40d5
Gerrit-Change-Number: 15006
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 10 Jan 2020 19:05:32 +0000
Gerrit-HasComments: Yes

Reply via email to