Adar Dembo has posted comments on this change.

Change subject:  fix compile error when compiling column_predicate-test.cc  In 
env : boost 1.57, centos 6.5, compile failed because of :  "operator<<: cannot 
bind lvalue to 'std::basic_ostream<char>&&'"  write a operator<< in 
column_predicate.h , compile succeed.
......................................................................


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/3233/3//COMMIT_MSG
Commit Message:

Line 8: :
      :  "operator<<: cannot bind lvalue to 'std::basic_ostream<char>&&'"
Can you provide the full compilation error so we can see exactly what's missing 
from where?


Line 7:  fix compile error when compiling column_predicate-test.cc
      :  In env : boost 1.57, centos 6.5, compile failed because of :
      :  "operator<<: cannot bind lvalue to 'std::basic_ostream<char>&&'"
      :  write a operator<< in column_predicate.h , compile succeed.
Please reformat this. Our git commit template is:

  <one line summary of the fix (required)>
  <empty line>
  <additional paragraphs providing more information (optional)>

All lines should be wrapped to 80 chars.


Line 12:   Author:    bruceSz song zhang<zsyuyizh...@gmail.com>
       :   Date:      Fri May 27 09:27:46 2016 +0800
       :  Change-Id: I1f6794b351e49d7a59c542a80161171b7e211093
Please remove these lines from the commit description.


http://gerrit.cloudera.org:8080/#/c/3233/3/src/kudu/common/column_predicate.h
File src/kudu/common/column_predicate.h:

Line 140:   // overload operator<< for optional<ColumnPredicate>
Does this overload need to be a class member? I don't see it accessing any 
class state; could it be a free function instead?


Line 141:   friend inline std::ostream& operator<<(std::ostream& out,
        :                                           
boost::optional<ColumnPredicate> const & v) {
Nit: format like this:

  friend inline std::ostream& operator<<(std::ostream& out,
                                         boost::optional<ColumnPredicate> 
const& v);

(I aligned the indentation of the continuation line and moved the '&' to be 
next to const.)


Line 143:     if ( out.good() ) {
Nit: no blank spaces between the if-condition and the parens. Below too.


Line 144:       if ( !v )
        :         out << "--";
        :       else
        :         out << ' ' << *v;
Nit: how about a ternary instead?

  out << v ? (<< ' ' << *v) : << "--";

Not sure if that works due to operator precedence, but maybe there's a way and 
I just got it wrong?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f9a9634eeccd86616be80b004ecce596155bb57
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: song bruce zhang <zsyuyizh...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to