Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make vendored logrus library reflect vendor.json #69

Merged
merged 3 commits into from
Jul 23, 2017
Merged

Conversation

mattbostock
Copy link
Owner

I ran govendor status to check the integrity of vendored packages and
found that the vendored copy of the logrus library did not match the
version pinned in vendor.json.

govendor gave the following error:

$ govendor status
The following packages are missing or modified locally:
        github.com/Sirupsen/logrus
Error: status failed for 1 package(s)

Additionally, remove the unused lowercase variant of logrus from
vendor.json. The URL for this library changed upstream to use
lowercase sirupsen instead of Sirupsen. The best way forward for now
is to continue to use the uppercase version as that's what's used by the
Prometheus libraries; mixed-case imports result in an import collision:

can't load package: package github.com/sirupsen/logrus: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

See this comment for more details:
sirupsen/logrus#570 (comment)


Also, fail CI tests if inconsistencies are found in vendored packages or if unused packages are found in the vendor/ directory.

I ran `govendor status` to check the integrity of vendored packages and
found that the vendored copy of the logrus library did not match the
version pinned in `vendor.json`.

govendor gave the following error:

    $ govendor status
    The following packages are missing or modified locally:
            github.com/Sirupsen/logrus
    Error: status failed for 1 package(s)

Additionally, remove the unused lowercase variant of logrus from
`vendor.json`. The URL for this library changed upstream to use
lowercase `sirupsen` instead of `Sirupsen`. The best way forward for now
is to continue to use the uppercase version as that's what's used by the
Prometheus libraries; mixed-case imports result in an import collision:

    can't load package: package github.com/sirupsen/logrus: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

See this comment for more details:
sirupsen/logrus#570 (comment)
This way is more readable.
Fail tests if libraries are missing from the vendor folder, if the
libraries in the vendor folder do not match the version in `vendor.json`
or if libraries are vendored but not actually used in the project.

Test for broken dependencies first in Travis so that tests will fail
fast as the `testdeps` target should return faster than `test` will.
@mattbostock mattbostock merged commit d2094ab into master Jul 23, 2017
@mattbostock mattbostock deleted the check_vendor branch July 23, 2017 17:02
@mattbostock mattbostock restored the check_vendor branch July 28, 2017 00:30
@mattbostock mattbostock deleted the check_vendor branch August 21, 2017 20:01
@mattbostock mattbostock restored the check_vendor branch August 29, 2017 11:47
@mattbostock mattbostock deleted the check_vendor branch August 29, 2017 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant