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.

Sign up for GitHub

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

Jump to bottom

Chore: Upgrade to golang 1.19 #585

Merged
merged 11 commits into from Dec 14, 2022
Merged

Chore: Upgrade to golang 1.19 #585

merged 11 commits into from Dec 14, 2022

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Dec 14, 2022

Update the minimum go version to 1.18

@ryantxu ryantxu requested a review from a team as a code owner December 14, 2022 01:07
@ryantxu ryantxu requested review from wbrowne, marefr and toddtreece and removed request for a team December 14, 2022 01:07
@toddtreece
Copy link
Member

@ryantxu #500 has some fixes for the lint issues

ryantxu
ryantxu commented Dec 14, 2022
View reviewed changes
.drone.yml Show resolved Hide resolved
toddtreece
toddtreece approved these changes Dec 14, 2022
View reviewed changes
Copy link
Member

@toddtreece toddtreece left a comment

Choose a reason for hiding this comment

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

approving, but i think @wbrowne or @marefr recommended testing with replace in grafana's go mod before merging these types of changes

ryantxu
ryantxu commented Dec 14, 2022
View reviewed changes
data/frame.go Outdated Show resolved Hide resolved
@ryantxu ryantxu mentioned this pull request Dec 14, 2022
Merged
@ryantxu
Copy link
Member Author

ryantxu commented Dec 14, 2022

@marefr does this "fix" #573?

Do we want to force 1.19 as minimum build version? or just make sure we build/test with 1.19?

ryantxu
ryantxu commented Dec 14, 2022
View reviewed changes
.golangci.toml
@@ -18,15 +18,13 @@ ignore-words = ["unknwon"]
disable-all = true
enable = [
"bodyclose",
"deadcode",
Copy link
Member Author

Choose a reason for hiding this comment

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

these were all deprecated and deleage to "unused"

marefr
marefr approved these changes Dec 14, 2022
View reviewed changes
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

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

LGTM great. Added some suggestions.

@marefr does this "fix" #573?

Do we want to force 1.19 as minimum build version? or just make sure we build/test with 1.19?

I don't see any problem with using 1.19 in go.mod and in builds. Am I missing something? If we do, then this will fix #573

data/frame.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@marefr
Copy link
Member

marefr commented Dec 14, 2022

approving, but i think @wbrowne or @marefr recommended testing with replace in grafana's go mod before merging these types of changes

Yes, that's normally a good idea.

ryantxu and others added 2 commits December 14, 2022 07:27
@ryantxu @marefr
Update data/frame.go
80ff37d 
Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
@ryantxu @marefr
Update go.mod
2bd0d7c 
Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
@ryantxu ryantxu changed the title Chore: Upgrade to golang 1.18 Chore: Upgrade to golang 1.19 Dec 14, 2022
@ryantxu ryantxu enabled auto-merge (squash) December 14, 2022 15:29
@ryantxu ryantxu merged commit 061f765 into main Dec 14, 2022
@ryantxu ryantxu deleted the use-go-18 branch December 14, 2022 15:29
@marefr marefr mentioned this pull request Dec 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toddtreece toddtreece toddtreece approved these changes

@marefr marefr marefr approved these changes

@wbrowne wbrowne Awaiting requested review from wbrowne wbrowne is a code owner automatically assigned from grafana/plugins-platform-backend

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ryantxu @toddtreece @marefr