build: add developer convenience targets#383
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds developer tooling targets to the Makefile for Kubernetes/Helm workflows and improved test reporting, plus a minor go.mod formatting/order adjustment.
Changes:
- Add Makefile targets for installing/uninstalling CRDs, Helm lint/template, and a
verifyaggregate target. - Add a
test-coveragetarget to run tests with race detection and coverage reporting. - Reorder/realign a dependency entry in
go.mod.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| go.mod | Minor dependency line reorder/formatting in require block. |
| Makefile | Adds CRD install/remove, Helm helpers, coverage target, and a verify aggregate. |
| install-crds: ## Install CRDs into the current Kubernetes cluster | ||
| kubectl apply -f manifests/charts/base/crds |
| test-coverage: ## Run tests with race detector and coverage | ||
| go test -race -v -coverprofile=coverage.out -coverpkg=./pkg/... ./pkg/... |
| $(GOLANGCI_LINT) run ./... | ||
|
|
||
| .PHONY: verify | ||
| verify: fmt vet lint test gen-check ## Run local verification before submitting PR |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several new Makefile targets (install-crds, uninstall-crds, test-coverage, verify, helm-template, and helm-lint) to streamline local development, testing, and verification. It also cleans up dependency sorting in go.mod. Feedback suggests adding gen-crd as a prerequisite to install-crds to ensure CRDs are up-to-date before installation, and restructuring the verify target to run sequentially using sub-make invocations to avoid race conditions during parallel execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| .PHONY: install-crds | ||
| install-crds: ## Install CRDs into the current Kubernetes cluster | ||
| kubectl apply -f manifests/charts/base/crds |
There was a problem hiding this comment.
The install-crds target applies CRD manifests from manifests/charts/base/crds. If these manifests have not been generated yet or are outdated, kubectl apply will fail or install stale CRDs. Adding gen-crd as a prerequisite ensures the CRDs are always generated and up-to-date before installation.
.PHONY: install-crds
install-crds: gen-crd ## Install CRDs into the current Kubernetes cluster
kubectl apply -f manifests/charts/base/crds
| .PHONY: verify | ||
| verify: fmt vet lint test gen-check ## Run local verification before submitting PR |
There was a problem hiding this comment.
Running gen-check at the end of verify has two issues:
- If the generated code is out of date,
fmt,vet,lint, andtestwill run on stale code first. - If
makeis run with parallel execution (e.g.,make -j verify), these targets will run concurrently. Sincegen-checkmodifies files (viagen-allandgo mod tidy) whilelint,vet, andtestare reading them, this will cause race conditions and flaky failures.
To ensure correctness and safety under parallel execution, we should run gen-check first and execute the verification steps sequentially using sub-make invocations.
.PHONY: verify
verify: ## Run local verification before submitting PR
$(MAKE) gen-check
$(MAKE) fmt
$(MAKE) vet
$(MAKE) lint
$(MAKE) test
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #383 +/- ##
===========================================
+ Coverage 47.57% 58.06% +10.49%
===========================================
Files 30 34 +4
Lines 2819 3181 +362
===========================================
+ Hits 1341 1847 +506
+ Misses 1338 1150 -188
- Partials 140 184 +44
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: CYJiang <googs1025@gmail.com>
googs1025
force-pushed
the
add-makefile-dev-targets
branch
from
870ceb2 to
d21f0dd
Compare
What this PR does
verifytarget for common pre-submit checks.test-coverageto mirror the GitHub coverage command.install-crdsanduninstall-crdsfor local CRD validation workflows.helm-templateandhelm-lintfor chart validation.go mod tidyordering change required bymake gen-check.Verification
make helpmake -n install-crds uninstall-crds helm-template helm-lint verify test-coveragemake helm-templatemake helm-lintGOCACHE=/private/tmp/agentcube-go-cache make gen-checkNote:
make test-coverageruns locally, but on macOS this repository currently fails inpkg/picod/TestSanitizePathbecause temporary paths normalize between/varand/private/var. The PR target matches the existing Ubuntu CI coverage command.