Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/20504 )
Change subject: KUDU-3371 [fs] make LBMCorruptor a base class ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/20504/2/src/kudu/fs/log_block_manager-test-util.cc File src/kudu/fs/log_block_manager-test-util.cc: http://gerrit.cloudera.org:8080/#/c/20504/2/src/kudu/fs/log_block_manager-test-util.cc@66 PS2, Line 66: > nit: Could we move the definition for class NativeMetadataLBMCorruptor to The implementation details is hided in .cpp designedly, the LBMCorruptor users are not needed to know the implamentation details. The motivation is keeping the .h files smaller, and they can still create LBMCorruptor object by LBMCorruptor:Create(..) freely. http://gerrit.cloudera.org:8080/#/c/20504/5/src/kudu/fs/log_block_manager-test-util.cc File src/kudu/fs/log_block_manager-test-util.cc: http://gerrit.cloudera.org:8080/#/c/20504/5/src/kudu/fs/log_block_manager-test-util.cc@195 PS5, Line 195: // NOLINT(clang-analyzer-core.NonNullParamChecker) > nit: Is this for avoiding lint-checking errors? Just curious why is it not Yes, these NOLINT comments are aim to mute the Tidy Bot complaints in PS1. http://gerrit.cloudera.org:8080/#/c/20504/5/src/kudu/fs/log_block_manager-test-util.cc@467 PS5, Line 467: n data_dirs_[rand_.Uniform(d > I see some methods of the class NativeMetadataLBMCorruptor are spread over Done At the beginning, keep the modified code in the place would be convient for reviewer to compare code side-by-side :) -- To view, visit http://gerrit.cloudera.org:8080/20504 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a1f2842598f46e88fbd08273d8fd19ed34a9cc5 Gerrit-Change-Number: 20504 Gerrit-PatchSet: 6 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Mon, 16 Oct 2023 16:11:00 +0000 Gerrit-HasComments: Yes
