Clean up go.mod #62
Assignees
Milestone
Comments
benjaminjkraft
added a commit
that referenced
this issue
Sep 8, 2021
In principle, it's not a problem to have test-only deps in your go.mod, because they won't end up in your importers' builds (and in newer Go versions may not even be downloaded. (Which is why there's no annotation to do so.) In practice, that doesn't really work for golangci-lint, because it doesn't really use semver (reasonably, in that any updated linter may break lint in your codebase). And we don't really need it other than to run the binary at a particular version. So now, I put it in its own go module. This requires a bit more throat-clearing to run it (we can't just `go run`), but it's not so bad and avoids anyone getting annoyed at us because we upgraded their golangci-lint for them. We could do the same for `internal/integration`, which adds quite a lot, including gqlgen, to our dependency tree, but it's not clear there's a need. Fixes #62. Issue: #62 Test plan: make check Reviewers: marksandstrom, steve, adam, miguel
benjaminjkraft
added a commit
that referenced
this issue
Sep 9, 2021
## Summary: In principle, it's not a problem to have test-only deps in your go.mod, because they won't end up in your importers' builds (and in newer Go versions may not even be downloaded. (Which is why there's no annotation to do so.) In practice, that doesn't really work for golangci-lint, because it doesn't really use semver (reasonably, in that any updated linter may break lint in your codebase). And we don't really need it other than to run the binary at a particular version. So now, I put it in its own go module. This requires a bit more throat-clearing to run it (we can't just `go run`), but it's not so bad and avoids anyone getting annoyed at us because we upgraded their golangci-lint for them. We could do the same for `internal/integration`, which adds quite a lot, including gqlgen, to our dependency tree, but it's not clear there's a need. Fixes #62. Issue: #62 ## Test plan: make check Author: benjaminjkraft Reviewers: dnerdy, StevenACoffman, benjaminjkraft, aberkan, MiguelCastillo Required Reviewers: Approved By: dnerdy, StevenACoffman Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint Pull Request URL: #80
|
Note to self if we ever want to revisit this further: apparently there's now a |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Right now it has a bunch of test-only deps, which is mildly annoying but hopefully not a huge deal, but also golangci-lint, which is bad because you really want that pinned to a specific minor version, and now pulling in genqlient might force you to upgrade it. (Arguably that's golangci-lint's problem, but I think it's just life when it comes to a lint tool; honestly we will have similar problems, see #63.) We should definitely fix that for golangci-lint, which is very doable since all we do is
go runit, we don't actually need to import it. Ideally we also fix it for all the test-only deps (cupaloy, testify, gqlgen), although the latter may be swimming too much upstream of what go.mod wants us to do. Maybe it's doable at least for gqlgen, which is by far the largest of these in terms of import graph, and is integration-test-only.The text was updated successfully, but these errors were encountered: