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

chore: Updates to Go 1.23 #399

Closed
wants to merge 7 commits into from
Closed

chore: Updates to Go 1.23 #399

wants to merge 7 commits into from

Conversation

lantoli
Copy link
Member

@lantoli lantoli commented Oct 1, 2024

Description

Updates to Go 1.23

Ticket: CLOUDP-276354

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Required Checklist:

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code

Further comments

@@ -20,8 +20,7 @@ jobs:
- name: lint
uses: golangci/golangci-lint-action@v6.1.0
with:
version: v1.57.1
args: --timeout=10m
Copy link
Member Author

Choose a reason for hiding this comment

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

time-out moved to linter config file

@@ -75,7 +75,8 @@ linters:
- errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. [fast: false, auto-fix: false]
# Upstream code intentionally have number of non exhaustive checks
# - exhaustive # check exhaustiveness of enum switch statements [fast: false, auto-fix: false]
- exportloopref # checks for pointers to enclosing loop variables [fast: false, auto-fix: false]
# - exportloopref # checks for pointers to enclosing loop variables [fast: false, auto-fix: false]
Copy link
Member Author

Choose a reason for hiding this comment

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

changed because:

WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar. 

exclude-rules:
- linters:
- gocritic
text: "^builtinShadow: shadowing of predeclared identifier: (min|max)"
Copy link
Member Author

Choose a reason for hiding this comment

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

added because errors like:

 Error: admin/model_index_options.go:65:6: builtinShadow: shadowing of predeclared identifier: max (gocritic)
  	var max int = 180
  	    ^

Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

From the readme

Note that atlas-sdk-go only supports the two most recent major versions of Go.

@lantoli
Copy link
Member Author

lantoli commented Oct 1, 2024

@gssbzn sorry I don't understand your comment in this context

@gssbzn
Copy link
Collaborator

gssbzn commented Oct 1, 2024

if you change the version in the go.mod, consumers need to also update to 1.23 and we would drop support to 1.22 which we shouldn't

go.mod Outdated
@@ -1,6 +1,6 @@
module go.mongodb.org/atlas-sdk/v20240805004

go 1.22.0
go 1.23
Copy link
Collaborator

Choose a reason for hiding this comment

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

1.23 is not a valid version see golang/go#62278

@lantoli
Copy link
Member Author

lantoli commented Oct 1, 2024

@gssbzn oh, i see, that's why I was asking AKO

so we have to be always in one version previous to the latest version?

@gssbzn
Copy link
Collaborator

gssbzn commented Oct 1, 2024

@lantoli lantoli closed this Oct 1, 2024
@lantoli lantoli reopened this Oct 1, 2024
@lantoli lantoli closed this Oct 1, 2024
@lantoli lantoli deleted the CLOUDP-276354 branch October 1, 2024 22:28
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