Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11498 )
Change subject: [tools] ksck checksums: Add KsckChecksummer class ...................................................................... Patch Set 8: (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: True, but this class gets deleted in the follow-up :) 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)) > Right, so my question was whether it makes sense to continue the outer for( Yeah, it doesn't matter at all because this part changes completely in the follow-up :) 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 Done -- 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: Wed, 03 Oct 2018 05:11:19 +0000 Gerrit-HasComments: Yes
