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 to go 1.17 as minimum, fix and update actions #30

Merged
merged 2 commits into from
Jan 14, 2023

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jan 14, 2023

changes in this PR:

  • update go versions to go1.17 (minimum), and latest (1.19.x)
  • fix incorrect go-versions (plural) in the matrix, and rename "platform" to "os" (which looks to be what we use elsewhere)
  • update github action versions
  • set permissions and timeout for github actions
  • switch to use the golangci-lint action (instead of using a container)
  • update golangci-lint to 1.50.1, which is needed for compatibility with current go versions
  • update how we use sudo for testing (see this tweet)
  • add a go mod tidy check


require (
github.com/sirupsen/logrus v1.4.2
github.com/vishvananda/netlink v1.1.0
github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df
golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527
)

require github.com/konsorten/go-windows-terminal-sequences v1.0.1 // indirect
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this indirect should go away if we update the minimum logrus version

@thaJeztah thaJeztah force-pushed the update_go_versions branch 2 times, most recently from 8e33f5a to a52c56e Compare January 14, 2023 13:24
@thaJeztah thaJeztah force-pushed the update_go_versions branch 4 times, most recently from cbdcafe to 2c17e23 Compare January 14, 2023 14:35
@thaJeztah thaJeztah marked this pull request as ready for review January 14, 2023 14:36
@thaJeztah
Copy link
Member Author

Next up, after #31 was merged

@AkihiroSuda @neersighted @dims PTAL

Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

Can we split this into separate commits for the go.mod change and the CI?

- name: Ensure IPVS module
run: sudo modprobe ip_vs
run: |
Copy link
Member

Choose a reason for hiding this comment

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

I don't think there's a need for the multiline string syntax when we only have one line

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yeah, I made the change as I was syncing with other repositories (made it easier to compare).

Happy to revert if you prefer though (let me know)

Copy link
Member

Choose a reason for hiding this comment

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

That justification makes sense; ultimately using composite actions among the repos that are very similar (e.g. these small modules) is probably something we should explore.

committed the result of:

    go mod tidy -go=1.17 -compat=1.13

Signed-off-by: Sebastiaan van Stijn <[email protected]>
- update go versions to go1.17, and latest (1.19.x)
- fix incorrect `go-versions` (plural) in the matrix, and rename "platform"
  to "os" (which looks to be what we use elsewhere)
- update github action versions
- set permissions and timeout for github actions
- switch to use the golangci-lint action (instead of using a container)
- update golangci-lint to 1.50.1, which is needed for compatibility
  with current go versions
- replace "golint" with "revive" as golint is deprecated.
- update how we use `sudo` for testing (see [this tweet][1])
- add a go mod tidy check

[1]: https://twitter.com/kolyshkin/status/1367697997979557897

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Member Author

Can we split this into separate commits for the go.mod change and the CI?

Ah, yes, can do! Updated 👍

@neersighted neersighted merged commit 23977fd into moby:master Jan 14, 2023
@thaJeztah thaJeztah deleted the update_go_versions branch January 14, 2023 15:21
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.

2 participants