You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2019/06/03 09:34:15 UTC
[kudu-CR] [java] Micro-improvements to DataGenerator
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13498
Change subject: [java] Micro-improvements to DataGenerator
......................................................................
[java] Micro-improvements to DataGenerator
Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
---
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
1 file changed, 30 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/13498/1
--
To view, visit http://gerrit.cloudera.org:8080/13498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
Gerrit-Change-Number: 13498
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [java] Micro-improvements to DataGenerator
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13498 )
Change subject: [java] Micro-improvements to DataGenerator
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java:
http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@85
PS2, Line 85: continue;
> Really curious about the motivation for this.
The following two general principles of readable code:
1. Minimize the amount of indentation, because each level of indentation is usually another level of control flow to keep in mind.
2. Early return (or break or continue) is good because it allows the reader to eliminate cases from consideration in the rest of the function or block.
Also, I didn't actually mean to submit this patch. I just made this change to help me understand this code better, and then forgot to undo it when submitting the follow-up.
--
To view, visit http://gerrit.cloudera.org:8080/13498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
Gerrit-Change-Number: 13498
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 03 Jun 2019 16:44:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Micro-improvements to DataGenerator
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13498 )
Change subject: [java] Micro-improvements to DataGenerator
......................................................................
[java] Micro-improvements to DataGenerator
Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
Reviewed-on: http://gerrit.cloudera.org:8080/13498
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
1 file changed, 30 insertions(+), 28 deletions(-)
Approvals:
Kudu Jenkins: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
Gerrit-Change-Number: 13498
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [java] Micro-improvements to DataGenerator
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13498 )
Change subject: [java] Micro-improvements to DataGenerator
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java:
http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@85
PS2, Line 85: continue;
> The following two general principles of readable code:
I am fine with the change, was just curious the motivation given it was non-functional and minimal impact.
--
To view, visit http://gerrit.cloudera.org:8080/13498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
Gerrit-Change-Number: 13498
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 03 Jun 2019 16:45:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Micro-improvements to DataGenerator
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13498 )
Change subject: [java] Micro-improvements to DataGenerator
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
File java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java:
http://gerrit.cloudera.org:8080/#/c/13498/2/java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java@85
PS2, Line 85: continue;
Really curious about the motivation for this.
--
To view, visit http://gerrit.cloudera.org:8080/13498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
Gerrit-Change-Number: 13498
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Mon, 03 Jun 2019 16:09:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Micro-improvements to DataGenerator
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13498
to look at the new patch set (#2).
Change subject: [java] Micro-improvements to DataGenerator
......................................................................
[java] Micro-improvements to DataGenerator
Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
---
M java/kudu-client/src/main/java/org/apache/kudu/util/DataGenerator.java
1 file changed, 30 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/13498/2
--
To view, visit http://gerrit.cloudera.org:8080/13498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I78eca7c6a4d8e0f52fe646a4157774b9123c8a4a
Gerrit-Change-Number: 13498
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>