Adar Dembo has posted comments on this change. Change subject: Add some path / env related helper functions ......................................................................
Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/5618/3/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 896: unique_ptr<char, FreeDeleter> wd(getcwd(NULL, 0)); Please ask a Kudu macOS-based dev to build this before merging. http://gerrit.cloudera.org:8080/#/c/5618/3/src/kudu/util/env_util-test.cc File src/kudu/util/env_util-test.cc: Line 65: Env* env = Env::Default(); You can use KuduTest's env_ member. Could you fix TestDiskSpaceCheck too? PS3, Line 79: We have to clean this up manually > If this path is part of test directory, do we still have to clean this up m Agreed with Dinesh; test_dir_ will be recursively cleaned up when the test finishes, and verifying DeleteRecursively() shouldn't be in scope for this test. http://gerrit.cloudera.org:8080/#/c/5618/3/src/kudu/util/env_util.cc File src/kudu/util/env_util.cc: Line 29: #include "kudu/gutil/gscoped_ptr.h" What's this for? -- To view, visit http://gerrit.cloudera.org:8080/5618 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia664708a09493923abbf2ff4e56e3d49c62cf97e Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
