impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3924: Ubuntu16 support
Date Mon, 08 Aug 2016 20:45:55 GMT
Matthew Jacobs has posted comments on this change.

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


Patch Set 4:

(4 comments)

Thanks, I think it makes sense. Just a few small cleanup things while you're touching these
headers.

As a reference:
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

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: #include <cmath>
please put this above boost


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.inline.h"
            : 
            : #include "runtime/decimal-value.inline.h"
This seems weird to me. Can you change this to the following and make sure it compiles, or
maybe someone else knows why we're not pulling in the actual header directly.

 
#include "runtime/raw-value.h"


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: #include <cmath>
Put this above the boost header to be safe


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 "util/metrics.h"
            : #include "util/collection-metrics.h"
            : #include "util/memory-metrics.h"
Can you clean this up while you're here, i.e. move these down with the other local includes?


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message