Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10151 )
Change subject: [tools] ksck improvements [6/n]: Refactor result handling ...................................................................... Patch Set 13: (4 comments) http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@a994 PS12, Line 994: > This is redundant- if FLAGS_consensus = false, then the ContainsKey on L801 Gotcha, thanks for the clarification! http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@194 PS12, Line 194: return master->FetchConsensusState(); : }); > I think this is actually consistent with the GSG, since this lambda is an a Yeah, I agree it's readable so I'm fine with it, so long as it's consistent within the PS. http://gerrit.cloudera.org:8080/#/c/10151/12/src/kudu/tools/ksck.cc@284 PS12, Line 284: r (const auto& entry : cluster_->tablet_servers()) { > I agree it's a lot but I think 4 argument continuation + 2 for inside block Yeah, I can buy that. http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/tool_action_cluster.cc File src/kudu/tools/tool_action_cluster.cc: http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/tool_action_cluster.cc@a146 PS11, Line 146: > Hmm don't see this in PS12 I think you missed this? -- To view, visit http://gerrit.cloudera.org:8080/10151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609 Gerrit-Change-Number: 10151 Gerrit-PatchSet: 13 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Mon, 30 Apr 2018 21:29:05 +0000 Gerrit-HasComments: Yes
