Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22491 )
Change subject: [java] code cleanup ...................................................................... Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/22491/7//COMMIT_MSG Commit Message: PS7: Could you share the information how you came up with this list of updates? If that was some automated tool, does it make sense to start using it in the build stages like currently existing 'spotbugs' and 'checkstyle'? http://gerrit.cloudera.org:8080/#/c/22491/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java: http://gerrit.cloudera.org:8080/#/c/22491/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@114 PS7, Line 114: this.frozen = row.frozen; Did you consider the possibility that it was an intent to keep the result 'unfrozen' when copying an instance of a frozen PartialRow? In other words, did you do any research using git history to tell that this wasn't intentional? http://gerrit.cloudera.org:8080/#/c/22491/7/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java: http://gerrit.cloudera.org:8080/#/c/22491/7/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@131 PS7, Line 131: outputFile.toPath() Is it going to be a NPE from here if close() is called twice? I don't see how the API protects against calling the close() method twice, and the prior version at least wouldn't throw NPE in such a case. -- To view, visit http://gerrit.cloudera.org:8080/22491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iedaf7ffb0689f4c190fd585e73687b9c6c668890 Gerrit-Change-Number: 22491 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Wed, 19 Feb 2025 18:16:42 +0000 Gerrit-HasComments: Yes
