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

Update dependencies #242

Merged
merged 2 commits into from
Aug 1, 2020
Merged

Update dependencies #242

merged 2 commits into from
Aug 1, 2020

Conversation

roidelapluie
Copy link
Member

Signed-off-by: Julien Pivotto [email protected]

Signed-off-by: Julien Pivotto <[email protected]>
@beorn7
Copy link
Member

beorn7 commented Jun 30, 2020

Eeeenteresting. Go 1.11 and go 1.12 create a slightly different go.mod, and that trips our tests. I guess that's happening because a dependency has different Go requirements for different versions? Not sure how to proceed from here. There are no build tags for go.mod files, I presume...

@brian-brazil
Copy link
Contributor

I thought we'd excluded some tests from go-specifc things. Was that just for linting?

Odd that it'd fail on 11&12 and not the surrounding versions.

@beorn7
Copy link
Member

beorn7 commented Jun 30, 2020

We have a test that makes sure that go mod tidy will not create any changes in the go.mod file.

However, as described above, it creates a diff for go 1.11 and go 1.12 because the checked in go.mod file is created with go 1.14. Go 1.10 and go 1.9 do not suffer because they have no module support at all.

I guess we could run the go mod tidy check only for the current go version. Just not sure if we should actually have different go.mod files for different go versions. That would be bad…

Signed-off-by: Julien Pivotto <[email protected]>
@roidelapluie
Copy link
Member Author

I have added the indirect dependency to the go.mod. Tests succeed now.

@simonpasquier
Copy link
Member

I thought we'd excluded some tests from go-specifc things. Was that just for linting?

Yes and for gofmt too.

@roidelapluie
Copy link
Member Author

Can we merge this?

@brian-brazil
Copy link
Contributor

👍

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

Successfully merging this pull request may close these issues.

4 participants