You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2016/06/02 20:23:32 UTC

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................

Avoid missing 'override' keyword warnings in raft_consensus-test.cc

In this test we're using gmock and the compiler spews out warnings
that the mocked methods are missing the 'override' keyword.

This just makes it so that we ignore that warning in that file.

Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
---
M src/kudu/consensus/CMakeLists.txt
1 file changed, 2 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <da...@cloudera.com>

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/1723/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 2:

we can will do (disregard the update of this patch)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/2029/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/1874/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3289/2/src/kudu/consensus/CMakeLists.txt
File src/kudu/consensus/CMakeLists.txt:

Line 140: set_source_files_properties(raft_consensus-test.cc
OK. fair enough on not upgrading, but maybe drop a comment here explaining why we are suppressing?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 1:

could we upgrade/patch gmock instead?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 2:

was looking into updating gmock and it seems like it has moved to a new project https://github.com/google/googletest which has a different layout (uses cmake instead of make) so updating this won't just be updating the zip on s3. Would you rather still do it or can we just ignore the warning for now?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 2: Verified+1

unrelated failure on org.kududb.client.TestTimeouts.org.kududb.client.TestTimeouts

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Avoid missing 'override' keyword warnings in raft_consensus-test.cc

In this test we're using gmock and the compiler spews out warnings
that the mocked methods are missing the 'override' keyword.

This just makes it so that we ignore that warning in that file.

Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Reviewed-on: http://gerrit.cloudera.org:8080/3289
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/consensus/CMakeLists.txt
1 file changed, 8 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has uploaded a new patch set (#3).

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................

Avoid missing 'override' keyword warnings in raft_consensus-test.cc

In this test we're using gmock and the compiler spews out warnings
that the mocked methods are missing the 'override' keyword.

This just makes it so that we ignore that warning in that file.

Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
---
M src/kudu/consensus/CMakeLists.txt
1 file changed, 8 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 1: Verified+1

Unrelated flake.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3289/2/src/kudu/consensus/CMakeLists.txt
File src/kudu/consensus/CMakeLists.txt:

Line 140: set_source_files_properties(raft_consensus-test.cc
> OK. fair enough on not upgrading, but maybe drop a comment here explaining 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 1:

https://github.com/google/googletest/issues/533 has some discussion here

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

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

Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3289/1/src/kudu/consensus/CMakeLists.txt
File src/kudu/consensus/CMakeLists.txt:

Line 141: set_source_files_properties(raft_consensus-test.cc
Nit: add a comment explaining why we're doing this.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes