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

Improve usability of Golangci-lint #3193

Merged
merged 9 commits into from
May 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ linters:
- ginkgolinter
- gofmt
- goimports
- govet
- misspell
- nakedret
- unconvert
Expand Down
2 changes: 1 addition & 1 deletion hack/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ The standard workflow is performed inside a helper container to normalize the bu
- `release-description`: generate a release announcement detailing changes between 2 commits (typically tags). Expects 'RELREF' and 'PREREF' to be set
- `test`: execute all tests (_NOTE:_ 'WHAT' is expected to match the go cli pattern for paths e.g. './pkg/...'. This differs slightly from rest of the 'make' targets)
- `test-unit`: Run unit tests.
- `test-lint`: Run gofmt and golint against src files
- `test-lint`: Run golangci-lint against src files
- `test-functional`: Run functional tests (in tests/ subdirectory).
- `vet`: lint all CDI packages

Expand Down
3 changes: 1 addition & 2 deletions hack/build/bazel-build-builder.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,13 @@ if ! git diff-index --quiet HEAD~1 hack/build/docker; then
if [ "${CDI_CONTAINER_BUILDCMD}" = "buildah" ]; then
(cd ${BUILDER_SPEC} && buildah build ${BUILDAH_PLATFORM_FLAG} --manifest ${BUILDER_MANIFEST} .)
buildah manifest push --all ${BUILDER_MANIFEST} docker://${BUILDER_MANIFEST}
DIGEST=$(podman inspect $(podman images | grep ${UNTAGGED_BUILDER_IMAGE} | grep ${BUILDER_TAG} | awk '{ print $3 }') | jq '.[]["Digest"]')
DIGEST=$(podman inspect $(podman images | grep ${UNTAGGED_BUILDER_IMAGE} | grep ${BUILDER_TAG} | awk '{ print $3 }') | jq '.[]["Digest"]')
else
(cd ${BUILDER_SPEC} && docker build --tag ${BUILDER_MANIFEST} .)
docker push ${BUILDER_MANIFEST}
DIGEST=$(docker images --digests | grep ${UNTAGGED_BUILDER_IMAGE} | grep ${BUILDER_TAG} | awk '{ print $4 }')
fi


echo "Image: ${BUILDER_MANIFEST}"
echo "Digest: ${DIGEST}"
fi
2 changes: 1 addition & 1 deletion hack/build/bazel-docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ volumes="-v ${BUILDER_VOLUME}:/root:rw,z"
mkdir -p "${HOME}/.docker"
volumes="$volumes -v ${HOME}/.docker:/root/.docker:ro,z"

if [[ $CDI_CRI = podman* ]] && [[ -f "${XDG_RUNTIME_DIR-}/containers/auth.json" ]]; then
if [[ $CDI_CRI == podman* ]] && [[ -f "${XDG_RUNTIME_DIR-}/containers/auth.json" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

So from what I understand, maybe do an eq instead would be better? I am afraid doing a single = is an assignment instead of a comparison.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not touch this, it's the result of the sh formatter. I simply ran make format before making any changes.

Suposedly they're the same thing https://stackoverflow.com/questions/20449543/shell-equality-operators-eq

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can still change it if you prefer so.

volumes="$volumes --mount type=bind,source=${XDG_RUNTIME_DIR-}/containers/auth.json,target=/root/.docker/config.json,readonly"
elif [[ -f "${HOME}/.docker/config.json" ]]; then
volumes="$volumes --mount type=bind,source=${HOME}/.docker/config.json,target=/root/.docker/config.json,readonly"
Expand Down
14 changes: 7 additions & 7 deletions hack/build/bazel-generate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ bazel run \
//:gazelle $@

if [[ "$@" =~ "vendor" ]]; then
bazel run \
--config=${ARCHITECTURE} \
-- @com_github_bazelbuild_buildtools//buildozer 'add clinkopts -lnbd' //$HOME/go/src/kubevirt.io/containerized-data-importer/vendor/libguestfs.org/libnbd/:go_default_library
bazel run \
--config=${ARCHITECTURE} \
-- @com_github_bazelbuild_buildtools//buildozer 'add copts -D_GNU_SOURCE=1' //$HOME/go/src/kubevirt.io/containerized-data-importer/vendor/libguestfs.org/libnbd/:go_default_library
bazel run \
--config=${ARCHITECTURE} \
-- @com_github_bazelbuild_buildtools//buildozer 'add clinkopts -lnbd' //$HOME/go/src/kubevirt.io/containerized-data-importer/vendor/libguestfs.org/libnbd/:go_default_library

bazel run \
--config=${ARCHITECTURE} \
-- @com_github_bazelbuild_buildtools//buildozer 'add copts -D_GNU_SOURCE=1' //$HOME/go/src/kubevirt.io/containerized-data-importer/vendor/libguestfs.org/libnbd/:go_default_library
fi
2 changes: 1 addition & 1 deletion hack/build/build-go.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ elif [ "${go_opt}" == "build" ]; then
rm -f ${outLink}
static_flag=""
if [ "$tgt" == "tools/cdi-containerimage-server" ]; then
static_flag="static"
static_flag="static"
fi
(
cd $tgt
Expand Down
4 changes: 2 additions & 2 deletions hack/build/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ ARCHITECTURE="${BUILD_ARCH:-$(uname -m)}"
HOST_ARCHITECTURE="$(uname -m)"
CDI_CRI="$(determine_cri_bin)"
if [ "${CDI_CRI}" = "docker" ]; then
CDI_CONTAINER_BUILDCMD=${CDI_CONTAINER_BUILDCMD:-docker}
CDI_CONTAINER_BUILDCMD=${CDI_CONTAINER_BUILDCMD:-docker}
else
CDI_CONTAINER_BUILDCMD=${CDI_CONTAINER_BUILDCMD:-buildah}
CDI_CONTAINER_BUILDCMD=${CDI_CONTAINER_BUILDCMD:-buildah}
fi
echo "CDI_CRI: ${CDI_CRI}, CDI_CONTAINER_BUILDCMD: ${CDI_CONTAINER_BUILDCMD}"
2 changes: 0 additions & 2 deletions hack/build/docker/builder/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ RUN \
source /etc/profile.d/gimme.sh && \
eval $(go env) && \
go install github.com/onsi/ginkgo/v2/ginkgo@v2.12.0 && \
go install golang.org/x/tools/cmd/goimports@latest && \
go install mvdan.cc/sh/cmd/shfmt@latest && \
go install github.com/mattn/goveralls@latest && \
go install golang.org/x/lint/golint@latest && \
go install github.com/rmohr/go-swagger-utils/swagger-doc@latest && \
go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.13.0 && \
go install github.com/securego/gosec/v2/cmd/gosec@latest && \
Expand Down
3 changes: 1 addition & 2 deletions hack/build/format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,4 @@ source "${script_dir}"/common.sh
source "${script_dir}"/config.sh

shfmt -i 4 -w ${CDI_DIR}/hack ${CLONER_MAIN}
goimports -w -local kubevirt.io ${CDI_DIR}/cmd/ ${CDI_DIR}/pkg/ ${CDI_DIR}/tests/
(cd ${CDI_DIR} && go vet $(go list ./... | grep -v -E "vendor|pkg/client" | sort -u))
cd ${CDI_DIR} && "${script_dir}/run-linters.sh" --fix
2 changes: 1 addition & 1 deletion hack/build/rpm-deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ bazel run \
--config=aarch64 \
//:bazeldnf -- rpmtree \
--public --nobest \
--name cdi_uploadserver_base_aarch64 --arch aarch64\
--name cdi_uploadserver_base_aarch64 --arch aarch64 \
--basesystem centos-stream-release \
${bazeldnf_repos} \
$centos_base \
Expand Down
20 changes: 0 additions & 20 deletions hack/build/run-lint-checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,5 @@
source hack/build/config.sh
source hack/build/common.sh

LINTABLE=(staging/src/kubevirt.io/containerized-data-importer-api pkg cmd tests tools)
ec=0
out="$(gofmt -l -s ${SOURCE_DIRS} | grep ".*\.go")"
if [[ ${out} ]]; then
echo "FAIL: Format errors found in the following files:"
echo "${out}"
ec=1
fi
for p in "${LINTABLE[@]}"; do
echo "running golint on directory: ${p}"
out="$(golint ${p}/...)"
if [[ ${out} ]]; then
echo "FAIL: following golint errors found:"
echo "${out}"
ec=1
fi
done

set -e
./hack/build/run-linters.sh

exit ${ec}
4 changes: 2 additions & 2 deletions hack/build/run-linters.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/sh -ex

GOLANGCI_VERSION="${GOLANGCI_VERSION:-v1.54.2}"
GOLANGCI_VERSION="${GOLANGCI_VERSION:-v1.58.0}"

go install "github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_VERSION}"

golangci-lint run --timeout=16m ./...
golangci-lint run --timeout=16m ./... "$@"
4 changes: 2 additions & 2 deletions hack/ci/prom_metric_linter.sh
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,6 @@ errors=$($CDI_CRI run -i "quay.io/kubevirt/prom-metrics-linter:$linter_image_tag

# Check if there were any errors, if yes print and fail
if [[ $errors != "" ]]; then
echo "$errors"
exit 1
echo "$errors"
exit 1
fi
14 changes: 7 additions & 7 deletions hack/common-funcs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@
#

get_latest_release() {
curl -s "https://api.github.com/repos/$1/releases/latest" | # Get latest release from GitHub api
grep '"tag_name":' | # Get tag line
sed -E 's/.*"([^"]+)".*/\1/' # Pluck JSON value (avoid jq)
curl -s "https://api.github.com/repos/$1/releases/latest" | # Get latest release from GitHub api
grep '"tag_name":' | # Get tag line
sed -E 's/.*"([^"]+)".*/\1/' # Pluck JSON value (avoid jq)
}

get_previous_y_release() {
curl -s "https://api.github.com/repos/$1/releases" |
grep '"tag_name":' |
sed -E 's/.*"([^"]+)".*/\1/' |
sort -V | grep -v rc | grep "$2" -B 100 | grep -v "$2" | tail -n 1 | xargs
curl -s "https://api.github.com/repos/$1/releases" |
grep '"tag_name":' |
sed -E 's/.*"([^"]+)".*/\1/' |
sort -V | grep -v rc | grep "$2" -B 100 | grep -v "$2" | tail -n 1 | xargs
}
1 change: 0 additions & 1 deletion hack/fossa.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,3 @@ if [[ "${CI:-}" == "true" ]]; then
fi
FOSSA_API_KEY="$(cat $FOSSA_TOKEN_FILE)" fossa analyze $FOSSA_OPTS
FOSSA_API_KEY="$(cat $FOSSA_TOKEN_FILE)" fossa test --timeout=1800

1 change: 1 addition & 0 deletions pkg/importer/vddk-datasource_arm64.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/url"

v1 "k8s.io/api/core/v1"

"kubevirt.io/containerized-data-importer/pkg/common"
)

Expand Down