[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-27 Thread Mike Percy (Code Review) 
Mike Percy has submitted this change and it was merged.

Change subject: Rename MiniClusterBase to MiniCluster
..


Rename MiniClusterBase to MiniCluster

Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Reviewed-on: http://gerrit.cloudera.org:8080/7273
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/internal_mini_cluster.h
R src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
5 files changed, 12 insertions(+), 12 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-27 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-27 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-26 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-23 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7273/1/src/kudu/integration-tests/mini_cluster.h
File src/kudu/integration-tests/mini_cluster.h:

PS1, Line 47: MiniCluster
> Sounds good to me. Looking forward to seeing your ServerBase --> Server ren
That one bothers me less since there is like one caller in the whole codebase...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-23 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7273/1/src/kudu/integration-tests/mini_cluster.h
File src/kudu/integration-tests/mini_cluster.h:

PS1, Line 47: MiniCluster
> A few reasons:
Sounds good to me. Looking forward to seeing your ServerBase --> Server rename 
in the near future. :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-23 Thread Mike Percy (Code Review) 
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: Rename MiniClusterBase to MiniCluster
..

Rename MiniClusterBase to MiniCluster

Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
---
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/internal_mini_cluster.h
R src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
5 files changed, 12 insertions(+), 12 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-23 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7273/1/src/kudu/integration-tests/mini_cluster.h
File src/kudu/integration-tests/mini_cluster.h:

PS1, Line 44: InternalMiniCluster
> this is the base for both impls right? (guess this comment belongs to the p
oops, fixed


PS1, Line 47: MiniCluster
> I'm also wondering the same thing (e.g. ServerBase, though that's not an ab
A few reasons:

(1) I'd like to move some constants in here and I think it's ugly to refer to 
e.g. MiniClusterBase::UNIQUE_LOOPBACK.

(2) Even though it has some implementation details and code sharing in it, I 
think of MiniCluster as more of an interface (see more in #3)

(3) When I think of classes named *Base I think of first having an interface 
definition, then an abstract base class called Base that implements the 
interface (to share code), and finally subclass specializations. If there is an 
intention to be able to refer to an object by the base class (as an interface) 
I think it's friendlier to avoid Base in the naming of that interface because 
Base implies you are referring to a specific implementation.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-23 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7273/1/src/kudu/integration-tests/mini_cluster.h
File src/kudu/integration-tests/mini_cluster.h:

PS1, Line 47: MiniCluster
> whats the motivation behind the rename? the base suffix usually refers to a
I'm also wondering the same thing (e.g. ServerBase, though that's not an 
abstract class per se).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-23 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7273/1/src/kudu/integration-tests/mini_cluster.h
File src/kudu/integration-tests/mini_cluster.h:

PS1, Line 44: InternalMiniCluster
this is the base for both impls right? (guess this comment belongs to the 
previous patch)


PS1, Line 47: MiniCluster
whats the motivation behind the rename? the base suffix usually refers to an 
"abstract" class, which this is, and we don't usually use the implementations 
interchangeably.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-22 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: Rename MiniClusterBase to MiniCluster
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Rename MiniClusterBase to MiniCluster

2017-06-22 Thread Mike Percy (Code Review) 
Hello Todd Lipcon, Alexey Serbin,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Rename MiniClusterBase to MiniCluster
..

Rename MiniClusterBase to MiniCluster

Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
---
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/internal_mini_cluster.h
R src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
5 files changed, 11 insertions(+), 11 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I40273d6ca7ac5fdc4cf2a6de374d374fe86cbf36
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Todd Lipcon