Adar Dembo has posted comments on this change.

Change subject: [util/monotime] added handy operators for MonoTime
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3999/1/src/kudu/util/monotime.h
File src/kudu/util/monotime.h:

Line 201:   /// than the specified one.
> Good catch!  I'll remove this modification since it could break backward-co
Right, without the operators I understand the motivation, but with them it 
doesn't seem as necessary.


http://gerrit.cloudera.org:8080/#/c/3999/2/src/kudu/util/monotime.h
File src/kudu/util/monotime.h:

Line 246:   bool operator==(const MonoTime& other) const;
I forgot, there's this bit from the Google style guide:

"Prefer to define non-modifying binary operators as non-member functions. If a 
binary operator is defined as a class member, implicit conversions will apply 
to the right-hand argument, but not the left-hand one. It will confuse your 
users if a < b compiles but b < a doesn't."

Looks like ==, !=, <, <=, >, and <= fit that classification; could you redefine 
them as non-member functions?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8a336bc79461adebfb2d5c136d71f90efcd36db
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to