wilfred-s commented on code in PR #227:
URL: https://github.com/apache/yunikorn-release/pull/227#discussion_r3439697945


##########
Makefile:
##########
@@ -118,95 +130,69 @@ $(SHELLCHECK_BIN):
        @curl -sSfL 
"https://github.com/koalaman/shellcheck/releases/download/$(SHELLCHECK_VERSION)/$(SHELLCHECK_ARCHIVE)"
 \
                | tar -x -J --strip-components=1 -C "$(SHELLCHECK_PATH)" 
"shellcheck-$(SHELLCHECK_VERSION)/shellcheck"
 
+# Install helm
+$(HELM_BIN):
+       @echo "installing helm $(HELM_VERSION)"
+       @mkdir -p "$(HELM_PATH)"
+       @curl -sSfL "https://get.helm.sh/$(HELM_ARCHIVE)" \
+               | tar -x -z --strip-components=1 -C "$(HELM_PATH)" 
"$(HELM_ARCHIVE_BASE)/helm"
+
 # Install golangci-lint
 $(GOLANGCI_LINT_BIN):
        @echo "installing golangci-lint v$(GOLANGCI_LINT_VERSION)"
        @mkdir -p "$(GOLANGCI_LINT_PATH)"
        @curl -sSfL 
"https://github.com/golangci/golangci-lint/releases/download/v$(GOLANGCI_LINT_VERSION)/$(GOLANGCI_LINT_ARCHIVE)"
 \
                | tar -x -z --strip-components=1 -C "$(GOLANGCI_LINT_PATH)" 
"$(GOLANGCI_LINT_ARCHIVEBASE)/golangci-lint"
 
-.PHONY: format
-# Run go fmt and goimports
-format:
-       @echo "running go fmt"
-       @"$(GO)" fmt ./...
-       go install golang.org/x/tools/cmd/goimports@latest
-       goimports -local 
"github.com/apache/yunikorn-core,github.com/apache/yunikorn-scheduler,github.com/apache/yunikorn-release"
 -w .
-
-.PHONY: lint
 # Run lint against the previous commit for PR and branch build
 # In dev setup look at all changes on top of master
 lint: $(GOLANGCI_LINT_BIN)
        @echo "running golangci-lint"
        @"${GOLANGCI_LINT_BIN}" run
 
+CHART_DIR := helm-charts/yunikorn
+helm_lint: $(HELM_BIN)
+       @echo "running helm lint"
+       @"$(HELM_BIN)" lint "${CHART_DIR}" -f "${CHART_DIR}/values.yaml"
+
 # Check scripts
-.PHONY: check_scripts
-ALLSCRIPTS := $(shell find . -not \( -path ./tools -prune \) -not \( -path 
./build -prune \) -name '*.sh')
+ALLSCRIPTS := $(shell find . -not \( -path ./"${TOOLS_DIR}" -prune \) -not \( 
-path ./"${BUILD_DIR}" -prune \) -name '*.sh')
 check_scripts: $(SHELLCHECK_BIN)
        @echo "running shellcheck"
        @"$(SHELLCHECK_BIN)" ${ALLSCRIPTS}
 
-.PHONY: license-check
 # This is a bit convoluted but using a recursive grep on linux fails to write 
anything when run
 # from the Makefile. That caused the pull-request license check run from the 
github action to
 # always pass. The syntax for find is slightly different too but that at least 
works in a similar
 # way on both Mac and Linux. Excluding all .git* files from the checks.
+LICENSE_CHECK_OUT := $(BUILD_DIR)/license-check.txt
 license-check:
        @echo "checking license headers:"
 ifeq (darwin,$(OS))
-       $(shell mkdir -p build && find -E . -not \( -path './.git*' -prune \) 
-not \( -path ./build -prune \) -not \( -path ./tools -prune \) -regex 
".*\.(go|sh|md|yaml|yml|mod)" -exec grep -L "Licensed to the Apache Software 
Foundation" {} \; > build/license-check.txt)
+       $(shell mkdir -p "${BUILD_DIR}" && find -E . -not \( -path './.git*' 
-prune \) -not \( -path ./"${BUILD_DIR}" -prune \) -not \( -path 
./"${TOOLS_DIR}" -prune \) -regex ".*\.(go|sh|md|yaml|yml|mod)" -exec grep -L 
"Licensed to the Apache Software Foundation" {} \; > "${LICENSE_CHECK_OUT}")
 else
-       $(shell mkdir -p build && find . -not \( -path './.git*' -prune \) -not 
\( -path ./build -prune \) -not \( -path ./tools -prune \) -regex 
".*\.\(go\|sh\|md\|yaml\|yml\|mod\)" -exec grep -L "Licensed to the Apache 
Software Foundation" {} \; > build/license-check.txt)
+       $(shell mkdir -p "${BUILD_DIR}" && find . -not \( -path './.git*' 
-prune \) -not \( -path ./"${BUILD_DIR}" -prune \) -not \( -path 
./"${TOOLS_DIR}" -prune \) -regex ".*\.\(go\|sh\|md\|yaml\|yml\|mod\)" -exec 
grep -L "Licensed to the Apache Software Foundation" {} \; > 
"${LICENSE_CHECK_OUT}")
 endif
-       @if [ -s "build/license-check.txt" ]; then \
+       @if [ -s "${LICENSE_CHECK_OUT}" ]; then \
                echo "following files are missing license header:" ; \
-               cat build/license-check.txt ; \
+               cat "${LICENSE_CHECK_OUT}" ; \
                exit 1; \
        fi
        @echo "  all OK"
 
-.PHONY: perf-tools
 perf-tools:
        @echo "Running perf-tools"
        @cd perf-tools && make build
        @cd ../
 
-# Check that we use pseudo versions in master
-.PHONY: pseudo

Review Comment:
   The psuedo check is only relevant for the core and k8shim master branches 
not for any other branch. It does certainly not apply for the release code.
   The check only looks at the SI in the core repo and core & Si in the k8shim 
repo. The release build moves to a directory link replacement and never uses 
the tags in source tar ball  generated.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to