Mike Percy has posted comments on this change.

Change subject: path_util: Add IsRelativePath() helper function
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5617/1/src/kudu/util/path_util.cc
File src/kudu/util/path_util.cc:

Line 40:   if (path.empty() || path[0] != '/') return true;
> I agreed with Todd.
My reasoning behind empty being relative is that if you were to concatenate a 
bunch of paths, and one was a directory that was empty, then we would consider 
that relative, and basically equivalent to ".".

Regarding why have this at all, I thought if we ever wanted to point to 
non-POSIX then it's one less thing scattered everywhere. So might as well add a 
helper that says what we mean. If the concern is ability to inline, I suppose I 
could make it a header-only static function.


-- 
To view, visit http://gerrit.cloudera.org:8080/5617
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieae26e50fb299fc9227cbc2d212bce6db0c11571
Gerrit-PatchSet: 1
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

Reply via email to