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

Upgrade to Go 1.13 #3362

Merged
merged 2 commits into from Nov 18, 2019
Merged

Conversation

lukedirtwalker
Copy link
Collaborator

@lukedirtwalker lukedirtwalker commented Nov 12, 2019

This also upgrades the GoConvey dependency as it is required for Go 1.13.

Fixes #3349
Fixes #3009


This change is Reviewable

@lukedirtwalker lukedirtwalker changed the title WIP Test Go1.13 Upgrage to Go 1.13 Nov 13, 2019
@lukedirtwalker lukedirtwalker changed the title Upgrage to Go 1.13 Upgrade to Go 1.13 Nov 13, 2019
@lukedirtwalker lukedirtwalker marked this pull request as ready for review November 13, 2019 12:09
Copy link
Contributor

@oncilla oncilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @karampok)

Copy link
Contributor

@karampok karampok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


WORKSPACE, line 364 at r1 (raw file):

    importpath = "github.com/smartystreets/goconvey",
    vcs = "git",
    remote = "https://github.com/kormat/goconvey.git",

I would like not to be in a personal space a dependency, but I suppose it is okay since we want to remove convey
(I was a bit surprised that our change was only the SoMsg :) )

Copy link
Collaborator Author

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


WORKSPACE, line 364 at r1 (raw file):

Previously, karampok (Konstantinos) wrote…

I would like not to be in a personal space a dependency, but I suppose it is okay since we want to remove convey
(I was a bit surprised that our change was only the SoMsg :) )

Yeah agreed. Also this has been here since a long time.

This is also requires an upgrade of GoConvey

Fixes scionproto#3349
Fixes scionproto#3009
Copy link
Contributor

@karampok karampok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@lukedirtwalker lukedirtwalker merged commit b9e0f86 into scionproto:master Nov 18, 2019
@lukedirtwalker lukedirtwalker deleted the pubGo1_13 branch November 18, 2019 11:42
lukedirtwalker added a commit to lukedirtwalker/scion that referenced this pull request Nov 19, 2019
Recently we saw SIG acceptance tests fail more often that usual.
Since the Go1.13 change (scionproto#3362) we saw in some SIGs problems with the TUN device.
Investigating further we came across golang/go#30624.
From there we saw that water library is very old and still uses os.OpenFile,
which was replaced in other similar libs to mitigate afformentioned Go issue.
lukedirtwalker added a commit that referenced this pull request Nov 19, 2019
Recently we saw SIG acceptance tests fail more often that usual.
Since the Go1.13 change (#3362) we saw in some SIGs problems with the TUN device.
Investigating further we came across golang/go#30624.
From there we saw that water library is very old and still uses os.OpenFile,
which was replaced in other similar libs to mitigate afformentioned Go issue.
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.

Upgrade to Go 1.13 Go: go version 1.12.x breaks goconvey tests
3 participants