gps icon indicating copy to clipboard operation
gps copied to clipboard

Adapt goimports' fast filepath.Walk impl for ListPackages

Open sdboyer opened this issue 8 years ago • 5 comments

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

sdboyer avatar Mar 07 '17 14:03 sdboyer

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?

jstemmer avatar Apr 01 '17 22:04 jstemmer

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.

sdboyer avatar Apr 01 '17 23:04 sdboyer

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.

jstemmer avatar Apr 01 '17 23:04 jstemmer

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.

sdboyer avatar Apr 02 '17 01:04 sdboyer

This issue was moved to golang/dep#419

fabulous-gopher avatar Apr 21 '17 14:04 fabulous-gopher