-
Notifications
You must be signed in to change notification settings - Fork 662
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: reduce repetition of go versions #1432
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,8 @@ HAS_GOLANGCI := $(shell ls _output/bin/golangci-lint 2> /dev/null) | |
GOFUMPT_VERSION := v0.4.0 | ||
HAS_GOFUMPT := $(shell command -v gofumpt 2> /dev/null) | ||
|
||
GO_VERSION := $(shell sed -En 's/^go (.*)$$/\1/p' go.mod) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From https://go.dev/ref/mod#go-mod-file-go:
Hopefully the syntax does not change in the future. In the worst case users will fail to pull the golang image. Can you test for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like unnecessary complexity for a local make command, no? let me see what other sig projects do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no real standard in other sig projects. pushed up a commit to apply your suggestion. let me know what you think |
||
|
||
# REGISTRY is the container registry to push | ||
# into. The default is to push to the staging | ||
# registry, not production. | ||
|
@@ -134,7 +136,7 @@ gen: | |
./hack/update-docs.sh | ||
|
||
gen-docker: | ||
$(CONTAINER_ENGINE) run --entrypoint make -it -v $(CURRENT_DIR):/go/src/sigs.k8s.io/descheduler -w /go/src/sigs.k8s.io/descheduler golang:1.22.3 gen | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is Edit: Oh, I see. It runs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some users reported having issues running gen, so this was a contribution to run it in docker. it was added here https://github.com/kubernetes-sigs/descheduler/pull/1189/files There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if it is still used by anyone |
||
$(CONTAINER_ENGINE) run --entrypoint make -it -v $(CURRENT_DIR):/go/src/sigs.k8s.io/descheduler -w /go/src/sigs.k8s.io/descheduler golang:$(GO_VERSION) gen | ||
|
||
verify-gen: | ||
./hack/verify-conversions.sh | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
#!/bin/bash | ||
|
||
# Copyright 2024 The Kubernetes Authors. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
# go::verify_version verifies the go version is supported by the project. | ||
# descheduler actively supports 3 versions, therefore 3 go versions are supported. | ||
go::verify_version() { | ||
GO_VERSION=($(go version)) | ||
|
||
if [[ -z $(echo "${GO_VERSION[2]}" | grep -E 'go1.20|go1.21|go1.22') ]]; then | ||
echo "Unknown go version '${GO_VERSION[2]}', skipping gofmt." | ||
exit 1 | ||
fi | ||
} |
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.
Nice: https://github.com/actions/setup-go?tab=readme-ov-file#getting-go-version-from-the-gomod-file