bradfitz / go-issue-mirror

[old] precursor to golang.org/x/build/maintner/godata
24 stars 2 forks source link

issues: Simplify code to find the root directory. #14

Closed dmitshur closed 7 years ago

dmitshur commented 7 years ago

build.Import, when given an blank srcDir and build.FindOnly mode, simply finds an existing directory within GOPATH workspaces, it does not check if there's an existing Go package there. (For reference, the previous import path "github.com/bradfitz/go-issue-mirror" also did not have an actual Go package inside, its directory contains README.md and no .go files.)

Additionally, when given an explicit full import path, the logic doesn't care if there are elements that begin with _, . or are equal to testdata. That only matters when expanding patterns or listing all Go packages.

So, there's no reason to use importPathToDir to find a parent directory, and then use filepath.Join to join that path with _data subdirectory. It's simpler to just use importPathToDir to directly resolve the target directory.

dmitshur commented 7 years ago

Sorry if I'm pestering you with minor improvements, but I want to see importPathToDir used in the simplest way. That way, if other people look at this code, they won't think that importPathToDir cannot be used to resolve arbitrary paths. It can. That's why I wanted to simplify this code.

dmitshur commented 7 years ago

Friendly ping here too, @bradfitz. See full rationale above. Thank you for reviewing the other PR!