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

Ignore .go files when checking for license/legal #384

Closed
wants to merge 1 commit into from

Conversation

jonboulle
Copy link

The current heuristic for license or legal files is a little broad
because it will inadvertently sweep up go source files that shouldn't be
vendored. As one example, when trying to vendor the package
github.com/camlistore/go4/errorutil, godep save also brings in the
file github.com/camlistore/go4/blob/master/legal/legal.go.

This patch changes the IsLicenseFile and IsLegalFile checks to
immediately return false if the filename ends in .go.

The current heuristic for license or legal files is a little broad
because it will inadvertently sweep up go source files that shouldn't be
vendored. As one example, when trying to vendor the package
`github.com/camlistore/go4/errorutil`, `godep save` also brings in the
file `github.com/camlistore/go4/blob/master/legal/legal.go`.

This patch changes the `IsLicenseFile` and `IsLegalFile` checks to
immediately return false if the filename ends in `.go`.
@freeformz
Copy link

This looks good, but what happens when a legal file get's dropped at github.com/camlistore/go4/legal/LICENSE.md ? AFAICT we'd include that one too when we probably shouldn't as errorutil doesn't depend on legal and shouldn't vendor anything from it.

@jonboulle
Copy link
Author

@freeformz indeed that is also a problem today (e.g.), but it's unrelated to this one - can we land this and fix that in a follow up?

@freeformz
Copy link

@jonboulle I'm suggesting that fixing the later will also fix this issue, but in a more complete fashion.

@client9
Copy link
Contributor

client9 commented Jan 5, 2016

Hi @jonboulle I wrote the original code here. My apologies that it's causing you troubles.

Just so I understand... is the problem just that godep just vendoring files it doesn't need to? Or is this causing some other issue for you as well.

Another way of doing this is adding a HasSuffix check of .txt, .rst, .md, .rtf, will cover 99.999% of license files out there (well I don't know exactly but my previous scanning suggested this covered most of them).

Regards,

n

@freeformz
Copy link

Thanks, but I am closing this PR because the repo is going to be archived.

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

Successfully merging this pull request may close these issues.

3 participants