Todd Lipcon has posted comments on this change. Change subject: KUDU-1673 [java client] more informative kudu-spark write error ......................................................................
Patch Set 5: (7 comments) this would need appropriate tests before it can be committed http://gerrit.cloudera.org:8080/#/c/5725/5//COMMIT_MSG Commit Message: Line 7: KUDU-1673 [java client] more informative kudu-spark write error can you add a paragraph to the commit message which explains a bit more what the change is? It's annoying to have to go check JIRA for details on the bug (even though it may feel redundant to "duplicate" the summary from the JIRA) PS5, Line 11: Initial commit : : Change-Id: I663dddbf1b01f9c3d90f92dd484fc1a0018243c5 please remove this redundant stuff from the message (seems like it's left over from a squash) This review seems to be using the 'I663ddd' change id, so leave that one in palce and delete the I935b one http://gerrit.cloudera.org:8080/#/c/5725/5/java/kudu-client/src/main/java/org/apache/kudu/client/WriteException.java File java/kudu-client/src/main/java/org/apache/kudu/client/WriteException.java: Line 1: package org.apache.kudu.client; style: add license Line 6: public class WriteException extends RuntimeException { I don't think we want to be in the business of throwing unchecked exceptions. This would also need an interface annotation and appropriate javadoc. Line 22: public WriteException(RowError[] errors) { hopefully this could be package-private, or annotated as private Line 23: super(); isn't this implicit? Line 46: for(int i = 0; i < n && i < errorList.size(); i++){ style: spacing before (, after ) -- To view, visit http://gerrit.cloudera.org:8080/5725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I663dddbf1b01f9c3d90f92dd484fc1a0018243c5 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: eric-maynard <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: eric-maynard <[email protected]> Gerrit-HasComments: Yes
