Todd Lipcon has posted comments on this change. Change subject: [java] KUDU-2013 re-acquire authn token if expired ......................................................................
Patch Set 12: (2 comments) http://gerrit.cloudera.org:8080/#/c/7250/12/java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java File java/kudu-client/src/main/java/org/apache/kudu/client/AuthnTokenReacquirer.java: Line 155: this.attempts = 0; this is now redundant with the field initializer http://gerrit.cloudera.org:8080/#/c/7250/12/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java: Line 36: @Nonnull nit: again would prefer not to have these random changes in this commit. I'm not gonna force you go to back and remove them, but it really makes reviewing harder when there are a bunch of changes like this (plus cherry-picking becomes very hard to resolve conflicts, etc) Unless this commit is actually changing semantics here such that we used to sometimes have these be nullable but now they are never null, or whatever. Seeing this change adds cognitive load for the reviewer thinking "ah, what is this patch changing about how ServerInfo is used/constructed/etc?" and distracts from the meat of what's actually changing. -- To view, visit http://gerrit.cloudera.org:8080/7250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-HasComments: Yes
