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 2016/12/01 07:45:17 UTC
[kudu-CR] [clock] minor clean-up on the Clock class
Alexey Serbin has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5303
Change subject: [clock] minor clean-up on the Clock class
......................................................................
[clock] minor clean-up on the Clock class
Modified signature of a couple of methods to pass Timestamp parameter
by a const reference. Added const specifiers for a few other methods.
This patch does not contain any functional changes.
Change-Id: I5f451a132855a6fe85fff176bfc111aa2295c71f
---
M src/kudu/server/clock.h
M src/kudu/server/hybrid_clock.cc
M src/kudu/server/hybrid_clock.h
M src/kudu/server/logical_clock.cc
M src/kudu/server/logical_clock.h
5 files changed, 26 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/5303/1
--
To view, visit http://gerrit.cloudera.org:8080/5303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f451a132855a6fe85fff176bfc111aa2295c71f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [clock] minor clean-up on the Clock class
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [clock] minor clean-up on the Clock class
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5303/1/src/kudu/server/clock.h
File src/kudu/server/clock.h:
PS1, Line 95: const Timestamp& t
> Interesting -- the other half of methods in Clock's interface passes Timest
yes, please make it so :)?
--
To view, visit http://gerrit.cloudera.org:8080/5303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f451a132855a6fe85fff176bfc111aa2295c71f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [clock] minor clean-up on the Clock class
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [clock] minor clean-up on the Clock class
......................................................................
Patch Set 1:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/5303/1/src/kudu/server/clock.h
File src/kudu/server/clock.h:
PS1, Line 95: const Timestamp& t
we've had this discussion in the past and we've decided back then that Timestamps are lightweight and should always be passed by value.
PS1, Line 101: const Timestamp& timestamp)
same
http://gerrit.cloudera.org:8080/#/c/5303/1/src/kudu/server/hybrid_clock.cc
File src/kudu/server/hybrid_clock.cc:
PS1, Line 301: ExternalConsistencyMode mode) const {
spurious line wrapping
PS1, Line 372: const Timestamp& t)
same
PS1, Line 387: uint64_t* now_usec,
: uint64_t* error_usec) const {
does this not fit in one line anymore?
PS1, Line 457: const Timestamp& timestamp
same
http://gerrit.cloudera.org:8080/#/c/5303/1/src/kudu/server/hybrid_clock.h
File src/kudu/server/hybrid_clock.h:
PS1, Line 114: const Timestamp& t)
see my comment on the header
PS1, Line 122: OVERRIDE
since you're here can you change these to 'override'?
PS1, Line 138: uint64_t micros, uint64_t logical_value);
spurious change
PS1, Line 169: uint64_t* now_usec,
: uint64_t* error_usec) const;
spurious line wrapping
http://gerrit.cloudera.org:8080/#/c/5303/1/src/kudu/server/logical_clock.cc
File src/kudu/server/logical_clock.cc:
PS1, Line 79: const Timestamp& t)
same here and elsewhere
http://gerrit.cloudera.org:8080/#/c/5303/1/src/kudu/server/logical_clock.h
File src/kudu/server/logical_clock.h:
PS1, Line 61: const Timestamp& t)
same
PS1, Line 65: const Timestamp& timestamp)
same
PS1, Line 72: virtual bool SupportsExternalConsistencyMode(
: ExternalConsistencyMode mode) const OVERRIDE {
spurious line wrapping
--
To view, visit http://gerrit.cloudera.org:8080/5303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f451a132855a6fe85fff176bfc111aa2295c71f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] [clock] minor clean-up on the Clock class
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has abandoned this change.
Change subject: [clock] minor clean-up on the Clock class
......................................................................
Abandoned
not needed
--
To view, visit http://gerrit.cloudera.org:8080/5303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: I5f451a132855a6fe85fff176bfc111aa2295c71f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] [clock] minor clean-up on the Clock class
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [clock] minor clean-up on the Clock class
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5303/1/src/kudu/server/clock.h
File src/kudu/server/clock.h:
PS1, Line 95: const Timestamp& t
> we've had this discussion in the past and we've decided back then that Time
Interesting -- the other half of methods in Clock's interface passes Timestamp by a reference. I think it would be nice at least to have some consistency there, no?
--
To view, visit http://gerrit.cloudera.org:8080/5303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5f451a132855a6fe85fff176bfc111aa2295c71f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes