Skip to content
This repository has been archived by the owner on Feb 3, 2018. It is now read-only.

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

Open
sdboyer opened this issue Mar 7, 2017 · 5 comments
Open

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

sdboyer opened this issue Mar 7, 2017 · 5 comments

Comments

@sdboyer
Copy link
Owner

sdboyer commented Mar 7, 2017

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

@jstemmer
Copy link
Contributor

jstemmer commented Apr 1, 2017

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
Copy link
Owner Author

sdboyer commented Apr 1, 2017

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
Copy link
Contributor

jstemmer commented Apr 1, 2017

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
Copy link
Owner Author

sdboyer commented Apr 2, 2017

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
Copy link

This issue was moved to golang/dep#419

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants