This PR adds an automated make help target that scans all Makefiles#4088
Closed
nios-x wants to merge 6 commits intoOWASP:mainfrom
Closed
This PR adds an automated make help target that scans all Makefiles#4088nios-x wants to merge 6 commits intoOWASP:mainfrom
nios-x wants to merge 6 commits intoOWASP:mainfrom
Conversation
github-actions
bot
added
docs
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds a generated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/Makefile (1)
101-107: Consider consistent comment style for help descriptions.The file uses two different styles for help annotations:
- Standalone comment above target (lines 101, 222):
##@categoryDescription- Inline comment (lines 112, 118):
target: ##@categoryDescriptionWhile the awk parser handles both, using a consistent style improves maintainability.
Option: Align with inline style used elsewhere
-## `@Data` Enrich datasets from source systems -enrich-data: \ +enrich-data: ## `@Data` Enrich datasets from source systems +enrich-data: enrich-data-deps +enrich-data-deps: \Alternatively, keep current style if standalone comments are preferred for multi-line dependency targets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/Makefile` around lines 101 - 107, The comment style is inconsistent; change standalone help comments (e.g., the block above the enrich-data target that currently reads "## `@Data` Enrich datasets from source systems" and any other standalone help comments) to the inline style used elsewhere by appending the descriptor to the target line (e.g., enrich-data: ## `@Data` Enrich datasets from source systems) so all targets follow the same "target: ## `@Category` Description" format; update the enrich-data target and other similarly styled targets (like github-enrich-issues, owasp-enrich-*) to use the inline help annotation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/Makefile`:
- Around line 101-107: The comment style is inconsistent; change standalone help
comments (e.g., the block above the enrich-data target that currently reads "##
`@Data` Enrich datasets from source systems" and any other standalone help
comments) to the inline style used elsewhere by appending the descriptor to the
target line (e.g., enrich-data: ## `@Data` Enrich datasets from source systems) so
all targets follow the same "target: ## `@Category` Description" format; update
the enrich-data target and other similarly styled targets (like
github-enrich-issues, owasp-enrich-*) to use the inline help annotation.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
CONTRIBUTING.mdMakefilebackend/Makefile
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Confidence score: 4/5
- This PR looks safe to merge; the main concern is a minor performance overhead rather than functional breakage.
Makefileuses a top-level:=that triggers a full repositoryfindduring parsing, so anymakeinvocation may incur extra overhead and could fail before unrelated targets run.- Pay close attention to
Makefile- top-levelfindruns during parse and adds global overhead.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Makefile">
<violation number="1" location="Makefile:7">
P2: Top-level `:=` executes the repository-wide `find` during Makefile parsing, so every `make` run incurs a full scan even when `help` is not invoked. This adds global overhead and can fail before unrelated targets run. Use deferred evaluation so the scan only happens for `help`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Makefile (1)
176-221: Extract the embedded AWK program into a dedicated script file.The target works, but this inline block is hard to maintain and already triggers
maxbodylength. Moving it toscripts/make-help.awkwould simplify future edits and reviews.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 176 - 221, The inline AWK program embedded in the help target should be moved to a dedicated script: create scripts/make-help.awk containing the AWK code (including the add_entry function and BEGIN/END blocks), make it executable (or include a proper #! /usr/bin/awk -f header), and then simplify the Makefile help target to invoke awk -f scripts/make-help.awk $(HELP_MAKEFILES); ensure the new script reproduces the same logic/variable names (add_entry, entries, order, seen, doc) and that HELP_MAKEFILES is still passed through.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Line 7: Change HELP_MAKEFILES to only include repository-tracked Makefiles by
using git to list them instead of find; replace the assignment of HELP_MAKEFILES
with a command like `$(shell git ls-files -- 'Makefile' '*/Makefile' | sort)` so
only repo-owned (tracked) Makefiles are discovered for `make help` rather than
scanning local/generated directories.
- Around line 9-12: The .PHONY declaration is missing explicit entries for the
implicit/default targets flagged by checkmake (e.g., all and test); update the
.PHONY line to include "all" and "test" and add a lightweight default "all"
target (e.g., depends on build or prints help) so the linter stops reporting
minphony noise; modify the Makefile's .PHONY list and add a simple all target
(and if desired a test target stub) to clearly declare these targets referenced
elsewhere.
---
Nitpick comments:
In `@Makefile`:
- Around line 176-221: The inline AWK program embedded in the help target should
be moved to a dedicated script: create scripts/make-help.awk containing the AWK
code (including the add_entry function and BEGIN/END blocks), make it executable
(or include a proper #! /usr/bin/awk -f header), and then simplify the Makefile
help target to invoke awk -f scripts/make-help.awk $(HELP_MAKEFILES); ensure the
new script reproduces the same logic/variable names (add_entry, entries, order,
seen, doc) and that HELP_MAKEFILES is still passed through.
Comment on lines
9
to
+12
| .PHONY: build clean check pre-commit prune run scan-images security-scan security-scan-code \ | ||
| security-scan-code-semgrep security-scan-code-trivy security-scan-images \ | ||
| security-scan-backend-image security-scan-frontend-image security-scan-zap \ | ||
| test update clean-trivy-cache | ||
| test update clean-trivy-cache help |
Contributor
There was a problem hiding this comment.
Resolve checkmake minphony noise with explicit declarations.
Static analysis flags all/test phony requirements here. Adding explicit entries (and a lightweight all default) keeps lint output clean and avoids ambiguity.
Proposed fix
.PHONY: build clean check pre-commit prune run scan-images security-scan security-scan-code \
security-scan-code-semgrep security-scan-code-trivy security-scan-images \
security-scan-backend-image security-scan-frontend-image security-scan-zap \
test update clean-trivy-cache help
+.PHONY: all test
+
+all: help📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .PHONY: build clean check pre-commit prune run scan-images security-scan security-scan-code \ | |
| security-scan-code-semgrep security-scan-code-trivy security-scan-images \ | |
| security-scan-backend-image security-scan-frontend-image security-scan-zap \ | |
| test update clean-trivy-cache | |
| test update clean-trivy-cache help | |
| .PHONY: build clean check pre-commit prune run scan-images security-scan security-scan-code \ | |
| security-scan-code-semgrep security-scan-code-trivy security-scan-images \ | |
| security-scan-backend-image security-scan-frontend-image security-scan-zap \ | |
| test update clean-trivy-cache help | |
| .PHONY: all test | |
| all: help |
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 9-9: Missing required phony target "all"
(minphony)
[warning] 9-9: Missing required phony target "test"
(minphony)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 9 - 12, The .PHONY declaration is missing explicit
entries for the implicit/default targets flagged by checkmake (e.g., all and
test); update the .PHONY line to include "all" and "test" and add a lightweight
default "all" target (e.g., depends on build or prints help) so the linter stops
reporting minphony noise; modify the Makefile's .PHONY list and add a simple all
target (and if desired a test target stub) to clearly declare these targets
referenced elsewhere.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Makefile">
<violation number="1" location="Makefile:7">
P2: `find` exclusions only match top-level paths and don’t prune traversal, so nested dependency/build folders are still walked and can pollute help output or slow `make help`. Use globbed paths with `-prune` to skip traversal of those directories entirely.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/Makefile (1)
5-12: Consider documentingbuild-frontend-local-imagein help output.Line 6 defines a public/useful target, but it won’t be shown by
make helpwithout a##description.Suggested diff
-build-frontend-local-image: +build-frontend-local-image: ## Build local frontend Docker image🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/Makefile` around lines 5 - 12, The make target build-frontend-local-image is public but not listed by make help; add a help description by adding a '##' comment for that target (e.g., add a line like "build-frontend-local-image: ## Build local frontend Docker image" or place "## Build local frontend Docker image" immediately above the target) so the existing make help parser will include build-frontend-local-image in the help output; update the Makefile around the build-frontend-local-image target accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 50-53: The Makefile prune target over-promises age scoping: the
target named "prune" claims "older than 72h" but the docker volume prune command
(docker volume prune -f) has no age filter and may remove recent volumes; update
the prune target so behavior matches the description by adding the same age
filter to the volume prune command (use the --filter 'until=72h' / --filter
'until=72h' syntax) or alternatively relax the target description to reflect
that volumes are pruned unconditionally—change either the description or modify
the docker volume prune invocation in the prune target to include the age filter
so all three prune commands are consistent.
---
Nitpick comments:
In `@frontend/Makefile`:
- Around line 5-12: The make target build-frontend-local-image is public but not
listed by make help; add a help description by adding a '##' comment for that
target (e.g., add a line like "build-frontend-local-image: ## Build local
frontend Docker image" or place "## Build local frontend Docker image"
immediately above the target) so the existing make help parser will include
build-frontend-local-image in the help output; update the Makefile around the
build-frontend-local-image target accordingly.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Makefilebackend/Makefilecspell/Makefilefrontend/Makefileinfrastructure/Makefile
Makefile
Outdated
Comment on lines
+50
to
+53
| prune: ## Prune Docker resources older than 72h | ||
| @docker builder prune --filter 'until=72h' -a -f | ||
| @docker image prune --filter 'until=72h' -a -f | ||
| @docker volume prune -f |
Contributor
There was a problem hiding this comment.
prune description over-promises age scoping.
Line 50 says “older than 72h”, but Line 53 prunes unused volumes without any age qualifier. This can surprise contributors by removing newer volumes too.
Suggested diff
-prune: ## Prune Docker resources older than 72h
+prune: ## Prune unused Docker resources (builders/images older than 72h)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 50 - 53, The Makefile prune target over-promises age
scoping: the target named "prune" claims "older than 72h" but the docker volume
prune command (docker volume prune -f) has no age filter and may remove recent
volumes; update the prune target so behavior matches the description by adding
the same age filter to the volume prune command (use the --filter 'until=72h' /
--filter 'until=72h' syntax) or alternatively relax the target description to
reflect that volumes are pruned unconditionally—change either the description or
modify the docker volume prune invocation in the prune target to include the age
filter so all three prune commands are consistent.
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Makefile">
<violation number="1" location="Makefile:50">
P2: The `prune` target description claims it prunes Docker resources "older than 72h", but `docker volume prune -f` removes **all** unused volumes regardless of age. This mismatch between the documented behavior and actual behavior could lead to unexpected data loss. Either add `--filter 'until=72h'` to the volume prune command, or update the description to accurately reflect that volumes are pruned without an age qualifier (e.g., `Prune unused Docker resources (builders/images older than 72h, all unused volumes)`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Resolves #4080
This PR adds an automated make help target that scans all Makefiles (root and subdirectories) and generates a categorized, human-readable command list based on documented targets.
Checklist
make check-testlocally: all warnings addressed, tests passedScreenshots
