Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11498 )
Change subject: [tools] ksck checksums: Add KsckChecksummer class ...................................................................... Patch Set 8: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/11498/8/src/kudu/tools/ksck_checksum.h File src/kudu/tools/ksck_checksum.h: http://gerrit.cloudera.org:8080/#/c/11498/8/src/kudu/tools/ksck_checksum.h@176 PS8, Line 176: ~TabletServerChecksumCallbacks() = default; nit: it would be nice to add a comment like the following: // Prevent instances of this class to be allocated on the stack. http://gerrit.cloudera.org:8080/#/c/11498/6/src/kudu/tools/ksck_checksum.cc File src/kudu/tools/ksck_checksum.cc: http://gerrit.cloudera.org:8080/#/c/11498/6/src/kudu/tools/ksck_checksum.cc@372 PS6, Line 372: if (queue->BlockingGet(&table_tablet)) > That means the queue is empty (since it won't block because it was shut dow Right, so my question was whether it makes sense to continue the outer for() cycle in that case or just break out of it. Probably, that doesn't matter. http://gerrit.cloudera.org:8080/#/c/11498/8/src/kudu/tools/ksck_checksum.cc File src/kudu/tools/ksck_checksum.cc: http://gerrit.cloudera.org:8080/#/c/11498/8/src/kudu/tools/ksck_checksum.cc@205 PS8, Line 205: nit: this extra space is not needed -- To view, visit http://gerrit.cloudera.org:8080/11498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2016936eaa26fd6b499783e7d5d8f404816b37fa Gerrit-Change-Number: 11498 Gerrit-PatchSet: 8 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 02 Oct 2018 23:51:45 +0000 Gerrit-HasComments: Yes
