Todd Lipcon has posted comments on this change. Change subject: [security] introduced crypto-test ......................................................................
Patch Set 10: (3 comments) http://gerrit.cloudera.org:8080/#/c/5798/10/src/kudu/security/crypto-test.cc File src/kudu/security/crypto-test.cc: Line 153: static const uint16_t kKeyBits[] = { 256, 512, 1024, 2048, 3072, 4096 }; why would this test behave any differetly depending on the number of bits? Line 178: static const uint16_t kKeyBits[] = { 256, 512, 1024, 2048, 3072, 4096 }; same Line 189: // Extract public part of the key could we just combine this and the above test case? I guess it violates the purist unit testers ideas of "one test per distinct bit of functionality" but it would also remove ~50 lines of code, and the fewer lines of code the better, IMO. -- To view, visit http://gerrit.cloudera.org:8080/5798 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibace36b6336b650879cf3f9139ccf562f82765a7 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
