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/07/01 00:39:27 UTC

[kudu-CR] [java] add BaseKuduTest.doSetup() method

Dan Burkert has posted comments on this change.

Change subject: [java] add BaseKuduTest.doSetup() method
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7336/4/java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
File java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java:

Line 96:   protected static int getDefaultTimeoutMs() {
I don't really understand how this change is related, why change all the call sites to call this instead of using the static field?


http://gerrit.cloudera.org:8080/#/c/7336/1/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java:

Line 117:     final int timeoutMs = getDefaultTimeoutMs();
> This is to avoid calling the method again and again -- I'm not sure whether
It's not necessary to make it final.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id283bcb6a6d109839d86932cb807a1da8156f6eb
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes