----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42773/#review116786 -----------------------------------------------------------
Fix it, then Ship it! src/common/command_utils.hpp (line 65) <https://reviews.apache.org/r/42773/#comment177876> can you s/sha512/shasum/ and add an algorithm parameter (type string). we can probably create sha512 method still, but will invoke the underlying shasum method. src/common/command_utils.hpp (lines 65 - 66) <https://reviews.apache.org/r/42773/#comment177878> s/filePath/file/ I mentioned this in a couple of reviews. Please don't forget next time:) src/common/command_utils.cpp (line 20) <https://reviews.apache.org/r/42773/#comment177879> Why do you need this? src/common/command_utils.cpp (lines 30 - 31) <https://reviews.apache.org/r/42773/#comment177880> Is it used? src/common/command_utils.cpp (line 173) <https://reviews.apache.org/r/42773/#comment177885> please make sure the parameter name is consistent with that in the header! src/common/command_utils.cpp (line 177) <https://reviews.apache.org/r/42773/#comment177882> First letter capitalized src/common/command_utils.cpp (line 178) <https://reviews.apache.org/r/42773/#comment177881> First letter capitalized. src/common/command_utils.cpp (line 187) <https://reviews.apache.org/r/42773/#comment177883> s/output// from shasum command src/common/command_utils.cpp (lines 193 - 194) <https://reviews.apache.org/r/42773/#comment177884> Ditto. src/tests/common/command_utils_tests.cpp (line 226) <https://reviews.apache.org/r/42773/#comment177886> s/DigestTest/ShasumTest/ src/tests/common/command_utils_tests.cpp (line 231) <https://reviews.apache.org/r/42773/#comment177887> do you need this temp var? src/tests/common/command_utils_tests.cpp (line 237) <https://reviews.apache.org/r/42773/#comment177888> s/sha512Future/sha512/ src/tests/common/command_utils_tests.cpp (lines 242 - 243) <https://reviews.apache.org/r/42773/#comment177889> Please don't split this string. You can use ``` // NOLINT(whitespace/line_length) ``` in the end to silence style checker. - Jie Yu On Jan. 27, 2016, 10:29 p.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42773/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2016, 10:29 p.m.) > > > Review request for mesos. > > > Repository: mesos > > > Description > ------- > > Added utility function to compute SHA512 digest. > > > Diffs > ----- > > src/common/command_utils.hpp PRE-CREATION > src/common/command_utils.cpp PRE-CREATION > src/tests/common/command_utils_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/42773/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jojy Varghese > >
