You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Michele Milesi (Code Review)" <ge...@cloudera.org> on 2019/11/18 17:28:31 UTC
[kudu-CR] Fixed Type.getTypeForName method. Original version uses only name() method to check Type name. Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails.
Michele Milesi has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14736
Change subject: Fixed Type.getTypeForName method. Original version uses only name() method to check Type name. Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails.
......................................................................
Fixed Type.getTypeForName method.
Original version uses only name() method to check Type name.
Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails.
Now the method uses both getName() and name() to check Type name.
Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
---
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
A java/kudu-client/src/test/java/org/apache/kudu/TestType.java
2 files changed, 43 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/14736/1
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 1
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14736/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14736/3//COMMIT_MSG@10
PS3, Line 10: Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails.
Nit: this line is too long; please wrap it.
http://gerrit.cloudera.org:8080/#/c/14736/3/java/kudu-client/src/main/java/org/apache/kudu/Type.java
File java/kudu-client/src/main/java/org/apache/kudu/Type.java:
http://gerrit.cloudera.org:8080/#/c/14736/3/java/kudu-client/src/main/java/org/apache/kudu/Type.java@199
PS3, Line 199: * @param name The DataType name. It accepts Type name (from getName() method) and ENUM name (from name() method).
Nit: also too long, please wrap.
http://gerrit.cloudera.org:8080/#/c/14736/3/java/kudu-client/src/test/java/org/apache/kudu/TestType.java
File java/kudu-client/src/test/java/org/apache/kudu/TestType.java:
http://gerrit.cloudera.org:8080/#/c/14736/3/java/kudu-client/src/test/java/org/apache/kudu/TestType.java@32
PS3, Line 32: @Rule
: public ExpectedException expectedException = ExpectedException.none();
:
Doesn't seem like you're using this in the test?
http://gerrit.cloudera.org:8080/#/c/14736/3/java/kudu-client/src/test/java/org/apache/kudu/TestType.java@41
PS3, Line 41:
Nit: leading whitespace here, please remove.
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 3
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
Gerrit-Comment-Date: Tue, 19 Nov 2019 05:30:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 7
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
Gerrit-Comment-Date: Thu, 21 Nov 2019 03:25:59 +0000
Gerrit-HasComments: No
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14736
to look at the new patch set (#3).
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
[Java] Fixed Type.getTypeForName method.
Original version uses only name() method to check Type name.
Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails.
Now the method uses both getName() and name() to check Type name.
Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
---
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
A java/kudu-client/src/test/java/org/apache/kudu/TestType.java
2 files changed, 53 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/14736/3
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 3
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14736
to look at the new patch set (#6).
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
[Java] Fixed Type.getTypeForName method.
Original version uses only name() method to check Type name.
Output from name() (inerrithed from Enum) method was different
from output from Type.getName() method (ie INT32 vs int32),
so the check fails.
Now the method uses both getName() and name() to check Type name.
Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
---
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
A java/kudu-client/src/test/java/org/apache/kudu/TestType.java
2 files changed, 53 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/14736/6
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 6
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Michele Milesi has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
Patch Set 6:
> (4 comments)
>
> Looks good to me, just a few nits.
About the throws tag, I've seen that other methods did not have it, do you think that may be useful fix all of the javadoc entries?
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 6
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
Gerrit-Comment-Date: Wed, 20 Nov 2019 14:32:36 +0000
Gerrit-HasComments: No
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Michele Milesi has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
Patch Set 3:
> Uploaded patch set 3: Patch Set 2 was rebased.
Forced a jenkins rebuild because previous build fails due to a NTP timeout.
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 3
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
Gerrit-Comment-Date: Mon, 18 Nov 2019 22:07:43 +0000
Gerrit-HasComments: No
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Michele Milesi has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
Patch Set 4:
> Build Failed
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/19635/ : FAILURE
TSAN phase failed even all test passed?
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 4
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
Gerrit-Comment-Date: Wed, 20 Nov 2019 08:29:44 +0000
Gerrit-HasComments: No
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Michele Milesi has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
Patch Set 1:
> (5 comments)
Case insensitive check may be OK, but it requires that new values name are the upper case version of name property value.
By now we do not have a assert that validate this rule.
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 1
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
Gerrit-Comment-Date: Mon, 18 Nov 2019 21:12:59 +0000
Gerrit-HasComments: No
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14736
to look at the new patch set (#2).
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
[Java] Fixed Type.getTypeForName method.
Original version uses only name() method to check Type name.
Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails.
Now the method uses both getName() and name() to check Type name.
Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
---
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
A java/kudu-client/src/test/java/org/apache/kudu/TestType.java
2 files changed, 53 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/14736/2
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 2
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14736
to look at the new patch set (#4).
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
[Java] Fixed Type.getTypeForName method.
Original version uses only name() method to check Type name.
Output from name() (inerrithed from Enum) method was different
from output from Type.getName() method (ie INT32 vs int32),
so the check fails.
Now the method uses both getName() and name() to check Type name.
Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
---
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
A java/kudu-client/src/test/java/org/apache/kudu/TestType.java
2 files changed, 51 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/14736/4
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 4
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
Patch Set 5:
(4 comments)
Looks good to me, just a few nits.
http://gerrit.cloudera.org:8080/#/c/14736/5/java/kudu-client/src/main/java/org/apache/kudu/Type.java
File java/kudu-client/src/main/java/org/apache/kudu/Type.java:
http://gerrit.cloudera.org:8080/#/c/14736/5/java/kudu-client/src/main/java/org/apache/kudu/Type.java@199
PS5, Line 199:
the
http://gerrit.cloudera.org:8080/#/c/14736/5/java/kudu-client/src/main/java/org/apache/kudu/Type.java@199
PS5, Line 199:
Nit: Trailing whitespace
http://gerrit.cloudera.org:8080/#/c/14736/5/java/kudu-client/src/main/java/org/apache/kudu/Type.java@200
PS5, Line 200:
the
http://gerrit.cloudera.org:8080/#/c/14736/5/java/kudu-client/src/main/java/org/apache/kudu/Type.java@201
PS5, Line 201: * @return a matching Type.
Can you add an @throws for the IllegalArgumentException?
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 5
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
Gerrit-Comment-Date: Wed, 20 Nov 2019 13:55:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] Fixed Type.getTypeForName method. Original version uses only name() method to check Type name. Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails.
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: Fixed Type.getTypeForName method. Original version uses only name() method to check Type name. Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails.
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14736/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14736/1//COMMIT_MSG@7
PS1, Line 7: Fixed Type.getTypeForName method.
Nit: Add [java] and a line break after the subject
http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/main/java/org/apache/kudu/Type.java
File java/kudu-client/src/main/java/org/apache/kudu/Type.java:
http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/main/java/org/apache/kudu/Type.java@197
PS1, Line 197: public static Type getTypeForName(String name) {
Can you add javadoc that notes that this accepts the ENUM name or the type name?
http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/main/java/org/apache/kudu/Type.java@199
PS1, Line 199: if (t.name().equals(name) || t.getName().equals(name)) {
Would it make sense to do a case insensitive check by using toLower or toUpper?
http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/test/java/org/apache/kudu/TestType.java
File java/kudu-client/src/test/java/org/apache/kudu/TestType.java:
http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/test/java/org/apache/kudu/TestType.java@37
PS1, Line 37: String name = Type.INT64.getName();
Can you also verify that `Type.INT64.name()` works too?
http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/test/java/org/apache/kudu/TestType.java@39
PS1, Line 39:
nit: whitespace
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 1
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 18 Nov 2019 19:55:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
[Java] Fixed Type.getTypeForName method.
Original version uses only name() method to check Type name.
Output from name() (inerrithed from Enum) method was different
from output from Type.getName() method (ie INT32 vs int32),
so the check fails.
Now the method uses both getName() and name() to check Type name.
Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Reviewed-on: http://gerrit.cloudera.org:8080/14736
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
A java/kudu-client/src/test/java/org/apache/kudu/TestType.java
2 files changed, 53 insertions(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 8
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14736
to look at the new patch set (#5).
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
[Java] Fixed Type.getTypeForName method.
Original version uses only name() method to check Type name.
Output from name() (inerrithed from Enum) method was different
from output from Type.getName() method (ie INT32 vs int32),
so the check fails.
Now the method uses both getName() and name() to check Type name.
Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
---
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
A java/kudu-client/src/test/java/org/apache/kudu/TestType.java
2 files changed, 51 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/14736/5
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 5
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
Patch Set 6:
Sure. Better documentation is always a good thing. Although it should probably be a separate follow on patch.
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 6
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>
Gerrit-Comment-Date: Wed, 20 Nov 2019 15:03:37 +0000
Gerrit-HasComments: No
[kudu-CR] [Java] Fixed Type.getTypeForName method.
Posted by "Michele Milesi (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14736
to look at the new patch set (#7).
Change subject: [Java] Fixed Type.getTypeForName method.
......................................................................
[Java] Fixed Type.getTypeForName method.
Original version uses only name() method to check Type name.
Output from name() (inerrithed from Enum) method was different
from output from Type.getName() method (ie INT32 vs int32),
so the check fails.
Now the method uses both getName() and name() to check Type name.
Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
---
M java/kudu-client/src/main/java/org/apache/kudu/Type.java
A java/kudu-client/src/test/java/org/apache/kudu/TestType.java
2 files changed, 53 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/14736/7
--
To view, visit http://gerrit.cloudera.org:8080/14736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100
Gerrit-Change-Number: 14736
Gerrit-PatchSet: 7
Gerrit-Owner: Michele Milesi <mi...@icteam.it>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michele Milesi <mi...@icteam.it>