Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17753 )
Change subject: [common] Update PartitionSchema::operator==() ...................................................................... Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition-test.cc File src/kudu/common/partition-test.cc: http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition-test.cc@1573 PS2, Line 1573: ASSERT_EQ(p > nit here and below for ASSERT_TRUE(... == ...): ASSERT_TRUE() might be repl Done http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition-test.cc@1573 PS2, Line 1573: ASSERT_EQ(partition_schema, partition_schema); : ASSERT_EQ(partition_schema_1, partition_schema_1); > Does it make sense to add Done http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition-test.cc@1579 PS2, Line 1579: // Clears th > nit here and below for ASSERT_FALSE(... == ...): I guess it would be possib Done http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition-test.cc@1585 PS2, Line 1585: er_1, > nit here and below: add a trailing space or drop the space in the beginning Done http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition.h File src/kudu/common/partition.h: http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition.h@155 PS2, Line 155: bool operator== > It seems the code uses mostly operator!=(const HashBucketSchema& rhs), so m Done http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition.h@155 PS2, Line 155: rhs) const { > nit: would 'rhs' be more concise here? Done http://gerrit.cloudera.org:8080/#/c/17753/2/src/kudu/common/partition.h@159 PS2, Line 159: seed ! > nit here and below: maybe, drop 'this'? Or that's something you want to be Done -- To view, visit http://gerrit.cloudera.org:8080/17753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I09c626ecdcbe7c0465e6483d6a23369fb7194bfb Gerrit-Change-Number: 17753 Gerrit-PatchSet: 3 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Thu, 05 Aug 2021 06:46:44 +0000 Gerrit-HasComments: Yes
