Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

prune: Handle testdata directories #1649

Closed
wants to merge 1 commit into from

Conversation

sigma
Copy link
Contributor

@sigma sigma commented Feb 7, 2018

What does this do / why do we need it?

"testdata" is special, as documented in commit
golang/go@2f9eeea

Therefore such a directory can be discarded when test files are.

This should fix #1580

What should your reviewer look out for in this PR?

Do you need help or clarification on anything?

Which issue(s) does this PR fix?

"testdata" is special, as documented in commit
golang/go@2f9eeea

Therefore such a directory can be discarded when test files are.
@sigma sigma requested a review from sdboyer as a code owner February 7, 2018 20:41
@jmank88
Copy link
Collaborator

jmank88 commented Feb 8, 2018

This seems straightforward, and I'm surprised it wasn't already included. Was there a specific reason not to remove testdata dirs? @sdboyer @ibrasho

Do we have to consider legal/license files buried under testdata?

@sigma
Copy link
Contributor Author

sigma commented Feb 8, 2018

I guess it was coincidentally covered by non-go in most cases, so maybe was not too visible?

As for "legal" files, I personally don't like the amount of currently preserved files in otherwise empty directories. We regularly end up with relatively deep hierarchies containing just a license file that does not apply to anything still present, which is confusing at best.
I'd be happier with a rule like "preserve legal files in directories that contain a used package" (when unused packages are pruned of course) or something like that, but that's definitely a separate discussion :)
In any case, I don't think we should see cases where legal files in testdata would cover more than a subset of the test data itself (granted, anything's possible)

@sdboyer
Copy link
Member

sdboyer commented Feb 21, 2018

It is allowed by the compiler to import directly from testdata dirs. Same as it's allowed to import from a _* or .*-shaped directory - but all three of these shapes are ignored, by default, by go list.

That's why we can't do this. The fix for #1580 is to work out the issue with the symlinks.

@sdboyer sdboyer closed this Feb 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dep should prune testdata directories if go-tests pruner is enabled
4 participants