-
Notifications
You must be signed in to change notification settings - Fork 161
Conversation
go.mod
Outdated
@@ -1,13 +1,17 @@ | |||
module github.com/duo-labs/webauthn | |||
|
|||
go 1.17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will set min required version to 1.17 - that's not good for a library!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting go 1.17
does not prevent users with earlier Go versions from successfully building packages from this module since we are not using any Go 1.17 specific language features yet.
In addition, Go 1.16 will soon reach its end-of-life when Go 1.18 is released in February 2022.
@@ -5,10 +5,10 @@ jobs: | |||
name: Build | |||
runs-on: ubuntu-latest | |||
steps: | |||
- name: Set up Go 1.13 | |||
uses: actions/setup-go@v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not use a matrix to test minimum required go version and latest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest commit now uses matrix to test on Go 1.16 and Go 1.17
@@ -1,3 +1,4 @@ | |||
//go:build go1.13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
@@ -1,3 +1,4 @@ | |||
//go:build !go1.13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
👍 |
Perhaps we bump this to 1.18 now that 1.17 is no longer supported and try to get it merged? |
Signed-off-by: Eng Zer Jun <[email protected]>
@aseigler Updated to Go 1.18. Please take a look again, thank you 😃 |
I think this is ready to merge @MasterKale unless anyone has objections? This is what is breaking actions currently. |
I just first-time contributor actions, I think those have to run? Mobile doesn't show that real well so I'll check later today when I get back in front of a computer. If everything looks fine there I'll merge. |
Merging despite test failures, it's @aseigler's fault if it breaks a bunch of stuff (and mine if it fixes them all) |
This PR introduces two changes:
go
directive ingo.mod
file by runninggo mod tidy -go=1.17
to enable module graph pruning and lazy module loading.go.yml
to Go 1.17Note 1: This PR does not prevent users with earlier Go versions from successfully building packages from this module.
Note 2: The additional
require
directive is used to record indirect dependencies for Go 1.17 or higher, see https://go.dev/ref/mod#go-mod-file-go.