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

fix(deps): update all modules #108

Merged
merged 1 commit into from
Aug 10, 2022
Merged

fix(deps): update all modules #108

merged 1 commit into from
Aug 10, 2022

Conversation

rahul2393
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

Looks generally good to me, with small question on the test version matrix.

@@ -8,7 +8,7 @@ jobs:
test:
strategy:
matrix:
go-version: [1.15.x, 1.16.x]
go-version: [1.15.x, 1.18.x]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we require go 1.17 as the minimum, so would it not make more sense to test with 1.17 and 1.18 here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried making it work similar to https://github.com/googleapis/google-cloud-go/blob/main/spanner/go.mod#L3, In client library too we do test with earliest(1.15) and latest(1.18) Go versions on each presubmit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But why is 1.15 earliest in this case? Aren't the other updates in this PR setting the minimum required version of Go to 1.17?

Copy link
Collaborator Author

@rahul2393 rahul2393 Aug 10, 2022

Choose a reason for hiding this comment

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

@rahul2393 rahul2393 merged commit 2d13f6d into main Aug 10, 2022
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.

None yet

2 participants