_From @sdboyer on March 7, 2017 14:44_
goimports has an optimized implementation of filepath.Walk() that we should adapt for ListPackages().
_Copied from original issue: sdboyer/gps#180_
_From @jstemmer on April 1, 2017 22:19_
I've had a quick look at the fastWalk implementation and it looks like it should be pretty easy to replace the current filepath.Walk implementation with this faster one. Some quick testing shows a consistent improvement in ListPackages performance, I can clean it up and prepare a PR.
Can we just copy the source into this repo with the licensing headers intact or is there something else (license-wise) that I should keep in mind?
_From @sdboyer on April 1, 2017 23:38_
Ugh, licenses. We're already trying to figure that out in the reverse direction - golang/dep#300. Maybe it's best to just wait until after we finish that, as the licenses should then be compatible.
Also, a note - we may need it to be somewhat different from goimports' version, as our symlink needs may be a bit different - see #157 and #177.
_From @jstemmer on April 1, 2017 23:48_
Sure, I'll hold off on sending it for now and will keep it around in a local branch until gps is merged into dep.
I was vaguely aware that symlinks were being discussed in a couple of places but tried to avoid those discussions so far. I'll have a look through those issues and see what needs to be changed, thanks for linking them.
_From @sdboyer on April 2, 2017 1:21_
Sure, I'll hold off on sending it for now and will keep it around in a local branch until gps is merged into dep.
SGTM
I was vaguely aware that symlinks were being discussed in a couple of places but tried to avoid those discussions so far.
That seems like a good act of self-care. I am _truly_ coming to despise symlinks.
@jstemmer ok, this one should be ready for your attention now 馃槃
Great! I'll move my branch over soon and see what else was left to be done wrt symlinks before sending a PR.
@jstemmer i think a direct merge may not be possible because there was a filter-branch op that changed commit history...but either way, shouldn't be too bad. thanks!
@jstemmer pinging, just to see if you maybe have time to look at this again 馃槃
I've been busy lately, but I'll try to find some time to pick this up soon.
/cc @karrick, re: godirwalk
I have a tested branch of dep that uses https://github.com/karrick/godirwalk in ListPackages. After #1084 is merged, I'll submit a PR for this issue as well.
Most helpful comment
Great! I'll move my branch over soon and see what else was left to be done wrt symlinks before sending a PR.