use filepath instead of path#188
Conversation
thaJeztah
requested review from
kolyshkin,
mrunalp,
rhatdan and
runcom
as code owners
Originally, this package was documented to be for paths separated by forward slashes, but I was recently pointed to the updated package description, which amended the documentation (see https://go-review.googlesource.com/32423) and added: > To manipulate operating system paths, use the path/filepath package. A later patch (https://go-review.googlesource.com/45653) tweaked the description again, adding to the `path/filepath` package: > To process paths such as URLs that always use forward slashes regardless of > the operating system, see the path package. And to the `path` package > This package does not deal with Windows paths with drive letters or backslashes; > to manipulate operating system paths, use the path/filepath package. I expect it to be "fine" in general to use the `path` package for Unix/Linux, but from both patches, the intent of the Go maintainers is to use `path/filepath` for anything file(system) related, so let's adapt to that. Signed-off-by: Sebastiaan van Stijn <[email protected]>
|
LGTM |
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
I am slightly more inclined towards using path rather than filepath, as path is somewhat simpler, and will definitely work fine on Linux (and this code is Linux-specific and will stay that way).
OTOH I don't see a reason to not use filepath, so LGTM.
|
Yup; I originally thought "path" was the way to go as well. Fun fact (but I need to give it another go), was that I did some benchmarking at some time, and Also if their intent is for |
3 participants
Originally, this package was documented to be for paths separated by forward slashes, but I was recently pointed to the updated package description, which amended the documentation (see https://go-review.googlesource.com/32423) and added:
A later patch (https://go-review.googlesource.com/45653) tweaked the description again, adding to the
path/filepathpackage:And to the
pathpackageI expect it to be "fine" in general to use the
pathpackage for Unix/Linux, butfrom both patches, the intent of the Go maintainers is to use
path/filepathfor anything file(system) related, so let's adapt to that.