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

fix: support 1.17 & 1.16.7 Go versions #9642

Merged
merged 8 commits into from
Aug 23, 2021
Merged

fix: support 1.17 & 1.16.7 Go versions #9642

merged 8 commits into from
Aug 23, 2021

Conversation

sspaink
Copy link
Contributor

@sspaink sspaink commented Aug 17, 2021

New Go version 1.17: https://golang.org/doc/devel/release#go1.17
New Go version 1.16.7: https://golang.org/doc/devel/release#go1.16
Deprecated 1.15.8

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Aug 17, 2021
@sspaink
Copy link
Contributor Author

sspaink commented Aug 18, 2021

Gross, so many files changed. There is a change with gofmt in 1.17 to help migrate to the use of //go:build as the build flag, which allows using logical operators. make vet was failing without running go fmt ./... first. More information in the release notes: https://tip.golang.org/doc/go1.17#gofmt

@sspaink sspaink requested a review from reimda August 18, 2021 15:46
@sspaink sspaink added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Aug 18, 2021
@sspaink sspaink mentioned this pull request Aug 18, 2021
Open
@reimda
Copy link
Contributor

reimda commented Aug 18, 2021

Let's hold off on this for telegraf 1.19.3. I'd like to understand better why //go:build is required now.

@srebhan
Copy link
Member

srebhan commented Aug 23, 2021

@reimda maybe the release notes help. :-)

srebhan
srebhan approved these changes Aug 23, 2021
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

LGTM.

@reimda
Copy link
Contributor

reimda commented Aug 23, 2021

More details on golang's transition plan are at https://go.googlesource.com/proposal/+/master/design/draft-gobuild.md#transition.

As I understand it, once we start switching to go:build, we no longer support go 1.15. 1.17 will be preferred but 1.16 will be supported. 1.15 will be unsupported but might still partially work because +build lines will be kept in sync.

The next transition step will be when we switch to 1.18. At that point 1.17 will still work but 1.16 and previous won't because +build lines will be removed.

reimda
reimda approved these changes Aug 23, 2021
View reviewed changes
Copy link
Contributor

@reimda reimda left a comment

Choose a reason for hiding this comment

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

Looks good to me. Make sure to announce to devs before you merge that they must be on go 1.16 or later. A quick note in #telegraf-dev would be fine.

@telegraf-tiger
Copy link
Contributor

Looks like new artifacts were built from this PR. Get them here!

Artifact URLs

@sspaink sspaink merged commit 3a7d9b6 into influxdata:master Aug 23, 2021
@sspaink sspaink deleted the updatego17 branch August 23, 2021 20:37
@sspaink sspaink mentioned this pull request Aug 31, 2021
Merged
@Juneezee Juneezee mentioned this pull request Sep 23, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reimda reimda reimda approved these changes

@srebhan srebhan srebhan approved these changes

Assignees
No one assigned
Labels
fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sspaink @reimda @srebhan