You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/06/05 22:50:36 UTC

[kudu-CR] WIP: Bump googletest

Dan Burkert has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7089

Change subject: WIP: Bump googletest
......................................................................

WIP: Bump googletest

WIP because the change to client-test is just to see if this fixes the
clang tidy issue.

Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
---
M src/kudu/client/client-test.cc
M thirdparty/build-definitions.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
4 files changed, 7 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/7089/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7089
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>

[kudu-CR] Bump googletest

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Bump googletest
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7089/2/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 330:       -DCMAKE_INSTALL_PREFIX=$PREFIX \
> We end up building gmock twice (once for shared and once for static); I pre
Correct, and yes I checked.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Bump googletest

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7089

to look at the new patch set (#2).

Change subject: Bump googletest
......................................................................

Bump googletest

The new clang tidy is issuing warnings about gtest macros, the new
version seems to fix the issue.

Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
---
M thirdparty/build-definitions.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
3 files changed, 6 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/7089/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7089
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Bump googletest

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: Bump googletest
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7089/2/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 330:       -DCMAKE_INSTALL_PREFIX=$PREFIX \
We end up building gmock twice (once for shared and once for static); I presume its install logic is smart enough for both libraries to exist in the destination prefix at the same time? And could you also check that both gmock and gtest headers are placed in the right location?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Bump googletest

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Bump googletest
......................................................................


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Bump googletest

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.

Change subject: Bump googletest
......................................................................


Bump googletest

The new clang tidy is issuing warnings about gtest macros, the new
version seems to fix the issue.

Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
Reviewed-on: http://gerrit.cloudera.org:8080/7089
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
---
M thirdparty/build-definitions.sh
M thirdparty/download-thirdparty.sh
M thirdparty/vars.sh
3 files changed, 6 insertions(+), 10 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1cf6eee7007857f66b4177d3bfed665d8702e37f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>