Update project makefile to align with other operators#503
Update project makefile to align with other operators#503fontivan merged 4 commits intorh-ecosystem-edge:mainfrom
Conversation
- Update shared telco5g-konflux library to latest - Add versions for the tools used in the repo - Add $LOCALBIN configuration - Add target for syncing git submodules - Add appropriate dependencies to tools/linting targets
WalkthroughMakefile refactor: rename ROOT_DIR→PROJECT_DIR, add LOCALBIN and tool-version knobs, default goal, submodule sync target, local tool downloads (yamllint/yq), rust-compile target, and PATH adjustments; telco5g-konflux submodule pointer advanced. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Make as make (Makefile)
participant Sub as sync-git-submodules
participant LocalBin as LOCALBIN (dir)
participant Download as download tasks
participant Tools as yamllint / yq
participant Rust as rust-compile / cargo
Dev->>Make: make <target> (e.g., all, yamllint, yq, rust-ci)
Make->>Sub: run sync-git-submodules (unless SKIP_SUBMODULE_SYNC=true)
Make->>LocalBin: mkdir -p $(LOCALBIN)
Make->>Download: download yamllint/yq using specified versions -> $(LOCALBIN)
Download-->>LocalBin: place binaries
Make->>Tools: execute yamllint / yq against files under $(PROJECT_DIR)
Make->>Rust: run rust-deps ($(PROJECT_DIR)/hack/rust-deps.sh) then cargo build --release --bin recert
note right of Make: PATH includes $(abspath $(LOCALBIN))
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
/cherry-pick release-4.19 |
|
@fontivan: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/cherry-pick release-4.18 |
|
@fontivan: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/cherry-pick release-4.17 |
|
/cherry-pick release-4.16 |
|
@fontivan: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/cherry-pick release-4.14 |
|
@fontivan: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Makefile (3)
6-11: Fix typo and align clean-up with LOCALBIN.
- Typo: “aboslute” → “absolute”.
- Since you introduced LOCALBIN, prefer cleaning that path instead of hardcoded bin.
Apply within this hunk:
-# If you are setting this externally then you must use an aboslute path +# If you are setting this externally then you must use an absolute pathAdditionally, update clean (outside this hunk):
clean: @rm -rf target - @rm -rf bin + @rm -rf $(LOCALBIN)
38-47: Add help description and sync before update; parallelize submodule update.
- Add “## …” so this target appears in
make help.- Run
git submodule sync --recursivebefore update.- Use
--jobs=0to speed up large trees.-.PHONY: sync-git-submodules -sync-git-submodules: +.PHONY: sync-git-submodules +sync-git-submodules: ## Sync git submodules (honors SKIP_SUBMODULE_SYNC=yes) @echo "Checking git submodules" @if [ "$(SKIP_SUBMODULE_SYNC)" != "yes" ]; then \ echo "Syncing git submodules"; \ - git submodule update --init --recursive; \ + git submodule sync --recursive; \ + git submodule update --init --recursive --jobs=0; \ else \ echo "Skipping submodule sync"; \ fi
57-61: Make file discovery robust; remove redundant OWNERS filter.
- OWNERS won’t match “-name '*.yaml'”; the exclusion is redundant.
- Add
-type fand handle spaces/newlines with -print0/xargs -0.- $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/tekton update-task-refs PIPELINE_FILES="$(shell find $(PROJECT_DIR)/.tekton -name '*.yaml' -not -name 'OWNERS' | tr '\n' ' ')" + $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/tekton update-task-refs \ + PIPELINE_FILES="$$(find $(PROJECT_DIR)/.tekton -type f \( -name '*.yaml' -o -name '*.yml' \) -print0 | xargs -0 -r printf '%s ')"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Makefile(2 hunks)telco5g-konflux(1 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 63-63: Target body for "yamllint" exceeds allowed length of 5 (8).
(maxbodylength)
[warning] 74-74: Target body for "yq" exceeds allowed length of 5 (6).
(maxbodylength)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Red Hat Konflux / recert-4-20-on-pull-request
- GitHub Check: build
🔇 Additional comments (7)
telco5g-konflux (2)
1-1: LGTM: submodule pointer advancedOnly the submodule commit SHA changed. No direct code impact here.
1-1: Submodule SHA verified
New SHA578fd8be859f52e6a14daade53c5260065a08620is reachable upstream and the commit range3af2560…578fd8bconfirms the delta – ensure the submodule’s changelog between these SHAs has been reviewed.Makefile (5)
4-4: ROOT dir detection looks solid.Trailing-slash stripping via patsubst is correct and avoids double slashes.
18-19: PATH precedence OK.Exporting LOCALBIN before PATH resolution is fine; no action needed.
49-55: Guard against missing lock files to avoid hard failures
Conditionally skip the filter step if the corresponding.konflux/lock-*/redhat.repofile is absent.konflux-filter-unused-redhat-repos: sync-git-submodules ## Filter unused repositories from redhat.repo files in both runtime and build lock folders @echo "Filtering unused repositories from runtime lock folder..." - $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/rpm-lock filter-unused-repos REPO_FILE=$(PROJECT_DIR)/.konflux/lock-runtime/redhat.repo + @if [ -f "$(PROJECT_DIR)/.konflux/lock-runtime/redhat.repo" ]; then \ + $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/rpm-lock filter-unused-repos REPO_FILE=$(PROJECT_DIR)/.konflux/lock-runtime/redhat.repo; \ + else \ + echo "No runtime redhat.repo; skipping."; \ + fi @echo "Filtering unused repositories from build lock folder..." - $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/rpm-lock filter-unused-repos REPO_FILE=$(PROJECT_DIR)/.konflux/lock-build/redhat.repo + @if [ -f "$(PROJECT_DIR)/.konflux/lock-build/redhat.repo" ]; then \ + $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/rpm-lock filter-unused-repos REPO_FILE=$(PROJECT_DIR)/.konflux/lock-build/redhat.repo; \ + else \ + echo "No build redhat.repo; skipping."; \ + fi
63-71: Extract yamllint download into helper target & invoke local binary
- Yamllint recipe body >5 lines; move the download step into a new
download-yamllint-localtarget and reduce theyamllinttarget to ≤5 lines.- Invoke the downloaded binary via
$(LOCALBIN)/yamllint—ensureLOCALBINis defined in the Makefile.- .yamllint.yaml already ignores
vendor/, so third-party content is excluded.
12-17: Pin versions + add integrity checks. IntroduceYAMLLINT_SHA256andYQ_SHA256variables and pass them asDOWNLOAD_YAMLLINT_SHA256andDOWNLOAD_YQ_SHA256to the telco5g-konflux downloader. Manually verify thattelco5g-konflux/scripts/downloadsupports these checksum flags before merging.
| yq: sync-git-submodules $(LOCALBIN)## Download yq | ||
| @echo "Downloading yq..." | ||
| $(MAKE) -C $(ROOT_DIR)/telco5g-konflux/scripts/download download-yq DOWNLOAD_INSTALL_DIR=$(ROOT_DIR)/bin | ||
| $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download \ | ||
| download-yq \ | ||
| DOWNLOAD_INSTALL_DIR=$(LOCALBIN) \ | ||
| DOWNLOAD_YQ_VERSION=$(YQ_VERSION) | ||
| @echo "Yq downloaded successfully." |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do the same for yq (maxbodylength) and prefer the local binary.
Mirror the yamllint refactor to keep recipes short and deterministic.
Apply within this hunk:
-.PHONY: yq
-yq: sync-git-submodules $(LOCALBIN)## Download yq
- @echo "Downloading yq..."
- $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download \
- download-yq \
- DOWNLOAD_INSTALL_DIR=$(LOCALBIN) \
- DOWNLOAD_YQ_VERSION=$(YQ_VERSION)
- @echo "Yq downloaded successfully."
+.PHONY: yq
+yq: download-yq-local ## Ensure yq is available locally
+ @echo "yq is available at $(LOCALBIN)/yq"Add helper target (outside this hunk):
+.PHONY: download-yq-local
+download-yq-local: sync-git-submodules $(LOCALBIN)
+ $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download download-yq DOWNLOAD_INSTALL_DIR=$(LOCALBIN) DOWNLOAD_YQ_VERSION=$(YQ_VERSION)🧰 Tools
🪛 checkmake (0.2.2)
[warning] 74-74: Target body for "yq" exceeds allowed length of 5 (6).
(maxbodylength)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
84-89: Make yq-format loop robust to spaces/newlines; constrain find; avoid grepUse -type f with -print0 and read -r -d '' to safely handle all filenames and remove grep dependency.
-yq-sort-and-format: yq ## Sort keys/reformat all YAML files in the repository - @echo "Sorting keys and reformatting YAML files..." - @find . -name "*.yaml" -o -name "*.yml" | grep -v -E "(telco5g-konflux/|target/|vendor/|bin/|\.git/)" | while read file; do \ - echo "Processing $$file..."; \ - yq -i '.. |= sort_keys(.)' "$$file"; \ - done +yq-sort-and-format: yq ## Sort keys/reformat all YAML files in the repository + @echo "Sorting keys and reformatting YAML files..." + @find . -type f \( -name "*.yaml" -o -name "*.yml" \) -print0 \ + | xargs -0 -I{} bash -c 'case "{}" in *"/telco5g-konflux/"*|*"/target/"*|*"/vendor/"*|*"/bin/"*|*"/.git/"*) ;; *) echo "Processing {}"; yq -i '"'"'.. |= sort_keys(.)'"'"' "{}" ;; esac'
♻️ Duplicate comments (1)
Makefile (1)
75-81: Mirror yamllint refactor for yq; prefer local binary; fix maxbodylengthSame rationale; use a helper target and keep recipe minimal.
Apply within this hunk:
-.PHONY: yq -yq: sync-git-submodules $(LOCALBIN)## Download yq - @echo "Downloading yq..." - $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download \ - download-yq \ - DOWNLOAD_INSTALL_DIR=$(LOCALBIN) \ - DOWNLOAD_YQ_VERSION=$(YQ_VERSION) - @echo "Yq downloaded successfully." +.PHONY: yq +yq: download-yq-local ## Ensure yq is available locally + @echo "yq ready at $(LOCALBIN)/yq"Add helper target (outside this hunk):
+.PHONY: download-yq-local +download-yq-local: sync-git-submodules $(LOCALBIN) + $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download download-yq DOWNLOAD_INSTALL_DIR=$(LOCALBIN) DOWNLOAD_YQ_VERSION=$(YQ_VERSION)
🧹 Nitpick comments (5)
Makefile (5)
6-11: LOCALBIN setup is good; ensure absolute PATH even if overriddenIf someone sets LOCALBIN to a relative path, PATH may prepend a relative entry. Recommend abspath on export.
Apply within PATH export hunk:
-export PATH := $(LOCALBIN):$(PATH) +export PATH := $(abspath $(LOCALBIN)):$(PATH)
12-16: Add checksum pins for downloaded toolsGuard against supply-chain risk by passing expected SHA256s to the download scripts (if supported) or verifying after download.
Example (assuming scripts accept checksum vars):
YAMLLINT_VERSION ?= 1.35.1 +YAMLLINT_SHA256 ?= <fill> YQ_VERSION ?= v4.45.4 +YQ_SHA256 ?= <fill>And pass them:
- DOWNLOAD_YAMLLINT_VERSION=$(YAMLLINT_VERSION) + DOWNLOAD_YAMLLINT_VERSION=$(YAMLLINT_VERSION) DOWNLOAD_YAMLLINT_SHA256=$(YAMLLINT_SHA256)- DOWNLOAD_YQ_VERSION=$(YQ_VERSION) + DOWNLOAD_YQ_VERSION=$(YQ_VERSION) DOWNLOAD_YQ_SHA256=$(YQ_SHA256)
18-19: Prefer single space around := for consistencyMinor style nit; Makefile commonly uses single spacing.
-export PATH := $(LOCALBIN):$(PATH) +export PATH := $(LOCALBIN):$(PATH)
64-71: Reduce yamllint recipe length to satisfy checkmake (maxbodylength)Extract download into a helper; keep target short and deterministic.
Apply within this hunk:
-.PHONY: yamllint -yamllint: sync-git-submodules $(LOCALBIN)## Download yamllint and lint YAML files in the repository - @echo "Downloading yamllint..." - $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download \ - download-yamllint \ - DOWNLOAD_INSTALL_DIR=$(LOCALBIN) \ - DOWNLOAD_YAMLLINT_VERSION=$(YAMLLINT_VERSION) - @echo "Running yamllint on repository YAML files..." - yamllint -c $(PROJECT_DIR)/.yamllint.yaml . - @echo "YAML linting completed successfully." +.PHONY: yamllint +yamllint: download-yamllint-local ## Lint YAML files in the repository + yamllint -c $(PROJECT_DIR)/.yamllint.yaml .Add helper target (outside this hunk):
+.PHONY: download-yamllint-local +download-yamllint-local: sync-git-submodules $(LOCALBIN) + $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download download-yamllint DOWNLOAD_INSTALL_DIR=$(LOCALBIN) DOWNLOAD_YAMLLINT_VERSION=$(YAMLLINT_VERSION)
98-103: Use PROJECT_DIR for rust-deps script for consistencyEnsures invocation works regardless of CWD.
- hack/rust-deps.sh + $(PROJECT_DIR)/hack/rust-deps.sh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Makefile(2 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 64-64: Target body for "yamllint" exceeds allowed length of 5 (8).
(maxbodylength)
[warning] 75-75: Target body for "yq" exceeds allowed length of 5 (6).
(maxbodylength)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Red Hat Konflux / recert-4-20-on-pull-request
- GitHub Check: build
🔇 Additional comments (6)
Makefile (6)
4-4: PROJECT_DIR detection LGTMAccurate, portable way to anchor paths to the repo root.
30-30: clean target LGTMCleans target/ and LOCALBIN predictably.
49-53: Konflux repo filtering target LGTMGood use of PROJECT_DIR and gated submodule sync.
57-61: Tekton task-refs update LGTMCorrectly uses -print0/xargs -0 and PROJECT_DIR.
129-131: rust-ci composition LGTMClear, ordered dependencies for CI.
133-136: help target LGTMAWK pattern matches your annotated “## ” descriptions across targets.
- Fix typo - Fix clean target to use the variable for the bin folder - Add help output for the sync task - Separate the yamllint download from the lint task - Fix absolute paths where appropriate - Make submodule sync more robust
- Ensure the code can compile with its up to date submodules as part of git ci
fontivan
force-pushed
the
sskeard/makefiles-updates
branch
from
5e25c73 to
e87e451
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Makefile (2)
38-47: Good: submodule sync now includesgit submodule sync --recursive.This addresses prior feedback and correctly honors SKIP_SUBMODULE_SYNC.
79-87: Refactor yq target: keep body short and reuse a helper (matches yamllint pattern).Fixes checkmake maxbodylength warning and prefers local binary deterministically.
Apply:
-.PHONY: yq -yq: sync-git-submodules $(LOCALBIN) ## Download yq - @echo "Downloading yq..." - $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download \ - download-yq \ - DOWNLOAD_INSTALL_DIR=$(LOCALBIN) \ - DOWNLOAD_YQ_VERSION=$(YQ_VERSION) - @echo "Yq downloaded successfully." +.PHONY: yq +yq: download-yq-local ## Ensure yq is available locally + @echo "yq is available at $(LOCALBIN)/yq" + +.PHONY: download-yq-local +download-yq-local: sync-git-submodules $(LOCALBIN) + $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download download-yq DOWNLOAD_INSTALL_DIR=$(LOCALBIN) DOWNLOAD_YQ_VERSION=$(YQ_VERSION)
🧹 Nitpick comments (2)
Makefile (2)
90-94: Avoid grep in the yq formatter; filter in find for correctness and speed.Prevents false positives and handles filenames with newlines/spaces safely.
Apply:
- @find . -name "*.yaml" -o -name "*.yml" | grep -v -E "(telco5g-konflux/|target/|vendor/|bin/|\.git/)" | while read file; do \ + @find . \( -path ./telco5g-konflux -o -path ./target -o -path ./vendor -o -path ./bin -o -path ./.git \) -prune -o \ + -type f \( -name '*.yaml' -o -name '*.yml' \) -print0 | while IFS= read -r -d '' file; do \ echo "Processing $$file..."; \ - yq -i '.. |= sort_keys(.)' "$$file"; \ + yq -i '.. |= sort_keys(.)' "$$file"; \ done
12-16: Optional: surface tool versions inhelpor atools-versionstarget.A tiny target printing YAMLLINT_VERSION/YQ_VERSION aids debugging and reproducing CI locally.
Example:
+.PHONY: tools-versions +tools-versions: ## Print pinned tool versions + @echo "yamllint: $(YAMLLINT_VERSION)" + @echo "yq: $(YQ_VERSION)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Makefile(4 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 80-80: Target body for "yq" exceeds allowed length of 5 (6).
(maxbodylength)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Red Hat Konflux / recert-4-20-on-pull-request
- GitHub Check: build
🔇 Additional comments (2)
Makefile (2)
64-76: Yamllint flow looks solid and deterministic.Local download helper + config path via PROJECT_DIR is clean.
103-108: CI flow is comprehensive; compile included.rust-compile gated behind rust-deps and part of rust-ci is good for parity with other operators.
Also applies to: 139-141
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
Makefile (3)
23-23: Default goal set to ‘all’. Good fix.Resolves earlier feedback about unintended default target.
39-48: Submodule sync improved (addsgit submodule sync --recursive).Correct ordering and SKIP_SUBMODULE_SYNC guard retained.
81-88: Reduce ‘yq’ target body length and prefer helper (fixes checkmake maxbodylength).Mirror the yamllint pattern; keeps recipes short and deterministic.
-.PHONY: yq -yq: sync-git-submodules $(LOCALBIN) ## Download yq - @echo "Downloading yq..." - $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download \ - download-yq \ - DOWNLOAD_INSTALL_DIR=$(LOCALBIN) \ - DOWNLOAD_YQ_VERSION=$(YQ_VERSION) - @echo "Yq downloaded successfully." +.PHONY: yq +yq: download-yq-local ## Ensure yq is available locally + @echo "yq is available at $(LOCALBIN)/yq" + +.PHONY: download-yq-local +download-yq-local: sync-git-submodules $(LOCALBIN) + $(MAKE) -C $(PROJECT_DIR)/telco5g-konflux/scripts/download download-yq DOWNLOAD_INSTALL_DIR=$(LOCALBIN) DOWNLOAD_YQ_VERSION=$(YQ_VERSION)
🧹 Nitpick comments (6)
Makefile (6)
18-19: Make shell robustness: set explicit shell and fail-fast flags.Helps catch errors in pipelines/loops across recipes.
Apply:
export PATH := $(abspath $(LOCALBIN)):$(PATH) export CARGO_TERM_COLOR := always +SHELL := bash +.SHELLFLAGS := -eu -o pipefail -c
90-97: Harden YAML format loop (exclude dirs in find; NUL-safe).Avoid grep, handle odd filenames, and stay within repo.
-yq-sort-and-format: yq ## Sort keys/reformat all YAML files in the repository - @echo "Sorting keys and reformatting YAML files..." - @find . -name "*.yaml" -o -name "*.yml" | grep -v -E "(telco5g-konflux/|target/|vendor/|bin/|\.git/)" | while read file; do \ - echo "Processing $$file..."; \ - yq -i '.. |= sort_keys(.)' "$$file"; \ - done +yq-sort-and-format: yq ## Sort keys/reformat all YAML files in the repository + @echo "Sorting keys and reformatting YAML files..." + @find . \ + \( -path "./telco5g-konflux/*" -o -path "./target/*" -o -path "./vendor/*" -o -path "./bin/*" -o -path "./.git/*" \) -prune -false -o \ + \( -type f \( -name "*.yaml" -o -name "*.yml" \) -print0 \) \ + | while IFS= read -r -d '' file; do \ + echo "Processing $$file..."; \ + yq -i '.. |= sort_keys(.)' "$$file"; \ + done
104-109: Make cargo build reproducible in CI.Use Cargo.lock and fail on lock drift.
-cargo build --release --bin recert +cargo build --release --locked --bin recert
123-127: Lock dependency graph during ‘check’.-cargo check +cargo check --locked
129-133: Fail CI on clippy warnings.-cargo clippy +cargo clippy -- -D warnings
135-139: Lock deps during tests (and broaden scope if applicable).-cargo test +cargo test --locked
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
-
Makefile(3 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 81-81: Target body for "yq" exceeds allowed length of 5 (6).
(maxbodylength)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
Makefile (11)
4-4: PROJECT_DIR detection looks correct.Uses
$(lastword $ (MAKEFILE_LIST)) and strips trailing slash—robust for sub-makes.
6-11: LOCALBIN target/setup is sane.Absolute-path requirement is stated; mkdir -p rule is fine.
12-17: Tool versions pinned.Good for reproducibility. Consider documenting update procedure in README later.
31-31: Clean now removes LOCALBIN.Keeps workspace tidy and aligned with new var.
51-56: Konflux repo filtering uses PROJECT_DIR and depends on sync.Good dependency wiring and path hygiene.
59-63: Tekton task ref update: robust file collection.Uses -print0/xargs -0; good for spaces. No action needed.
65-73: yamllint download target is clean and reusable.Nice separation; mirrors best practice.
74-77: yamllint target prefers local config and binary.Looks good.
111-115: rust-deps invocation ok.Consider
bash -euin the script itself for robustness (out of scope here).If desired, I can open a follow-up to harden hack/rust-deps.sh with
set -euo pipefail.
117-121: fmt is fine as-is. Optional: enforce edition-specific config.No change required.
141-142: rust-ci composition looks good.Order ensures toolchain before fmt/check/clippy/test/build.
|
/retest |
|
@fontivan: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fontivan, omertuc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@fontivan: new pull request created: #510 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: new pull request created: #511 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: new pull request created: #512 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: new pull request created: #513 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@fontivan: new pull request created: #514 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
3 participants
Summary by CodeRabbit
New Features
Refactor
Chores