-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
*: remove old-style build directives #111156
Conversation
pkg/col/coldata/BUILD.bazel
Outdated
@@ -12,12 +12,14 @@ go_library( | |||
"nulls.go", | |||
"testutils.go", | |||
"vec.go", | |||
"vec_tmpl.go", |
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.
@yuzefovich any idea why this file wasn't her and/or why dev gen bazel
suddenly decided to add it now?
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.
I reverted the changes to the coldata/colexec stuff for now.
"//conditions:default": [], | ||
}), | ||
"@org_golang_x_sys//unix", | ||
], |
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.
@rickystewart any idea why gazelle made this change?
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.
Ah, it feels like the old-style tag was preventing it from understanding that we don't build that file on all these platforms.
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.
Ah, or it's the other way, it used to be able to exclude the dependency depending on the tag :/
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.
I'm guessing our gazelle version predates this change bazelbuild/bazel-gazelle#1243
The newer `go:build` directives have been supported since go 1.17, and the old ones have been deprecated in go 1.18. This commit removes all old-style directives. Epic: none Release note: None
f479f5d
to
3524294
Compare
I tried to update Gazelle, and (after working through a couple of issues) got stuck at this error:
Will close this PR for now and reopen when gazelle supports the |
The newer
go:build
directives have been supported since go 1.17, and the old ones have been deprecated in go 1.18. This commit removes all old-style directives.Epic: none
Release note: None