search

sdboyer / gps

your dependencies have arrived
MIT License
270 stars 24 forks source link

Adapt goimports' fast filepath.Walk impl for ListPackages #180

Open sdboyer opened 7 years ago

sdboyer commented 7 years ago

goimports has an optimized implementation of filepath.Walk() that we should adapt for ListPackages().

jstemmer commented 7 years ago

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?

sdboyer commented 7 years ago

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.

jstemmer commented 7 years ago

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.

sdboyer commented 7 years ago

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.

fabulous-gopher commented 7 years ago

This issue was moved to golang/dep#419