Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-3924: Ubuntu16 support
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3800/4/be/src/exprs/cast-functions-ir.cc
File be/src/exprs/cast-functions-ir.cc:

Line 18: 
> please put this above boost
Done


http://gerrit.cloudera.org:8080/#/c/3800/4/be/src/runtime/raw-value-ir.cc
File be/src/runtime/raw-value-ir.cc:

PS4, Line 15: #include "runtime/raw-value.h"
            : 
            : #include <cmath>
> This seems weird to me. Can you change this to the following and make sure 
The reason, of course, to include the ".inline" files is that it otherwise 
won't compile if you use any of the functions defined there. It happens to be 
the case here that we're not using any of the functions defined in 
"raw-value.inline.h", so it can be removed. (this necessitates adding some 
imports to this file that it was getting from there, but it still reduces the 
compilation dependencies a little so it seems worth it).


http://gerrit.cloudera.org:8080/#/c/3800/4/be/src/runtime/raw-value.inline.h
File be/src/runtime/raw-value.inline.h:

PS4, Line 22: 
> Put this above the boost header to be safe
Done


http://gerrit.cloudera.org:8080/#/c/3800/4/be/src/util/metrics-test.cc
File be/src/util/metrics-test.cc:

PS4, Line 14: 
            : #include <cmath>
            : #include <gtest/gtest.h>
            : #include <boost/scoped_ptr.hpp>
> Can you clean this up while you're here, i.e. move these down with the othe
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa1520c1e4aa4175468ac342b14c1262fa745f7a
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Michael Brown <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-HasComments: Yes

Reply via email to