impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
Date Tue, 28 Mar 2017 22:06:46 GMT
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 9:

(2 comments)

Thanks for the quick reviews!

http://gerrit.cloudera.org:8080/#/c/5688/9/be/src/util/time.h
File be/src/util/time.h:

Line 36:   return ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec;
> are you sure impala doesn't have a commit that introduced GetMonoTimeNanos(
I think we did add it for MacOS (see https://github.com/apache/incubator-impala/commit/aeadd326a982af0f2ac8d55e425248a8bc7babfe),
and that added GetMonoTimeNanos() for both platforms. I took the view that it's easier to
maintain Impala-side dependencies for now, and that the MacOS port is effectively dead at
this point.


http://gerrit.cloudera.org:8080/#/c/5688/9/cmake_modules/kudu_cmake_fns.txt
File cmake_modules/kudu_cmake_fns.txt:

Line 64:   endif()
> what do these two changes do and why are they needed now?
The first change adds custom compile flags to this target. This is needed to suppress some
spurious warnings.

The second change isn't strictly needed here, but since gutil is the first user of ADD_EXPORTABLE_LIBRARY
I figured I'd include the fix at the earliest point. This allows CMake to properly handle
transitive dependencies, so we don't need to 'flatten' all the deps of, e.g., kudu_util when
linking against it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message