fix(learn): address PR #51 review findings - #53
Conversation
Skip Makefile/Justfile assignments, avoid inventing next lint or FastAPI for plain main.py, preserve make/just bodies on Node, gate cargo/go run on real binaries, and prefix Pipenv commands. Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Co-authored-by: Aanish Bhirud <baanish@users.noreply.github.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| continue; | ||
| }; | ||
| // Skip variable assignments (`x := y`, `x ?= y`, `x ::= y`). | ||
| if rest.starts_with('=') { |
There was a problem hiding this comment.
WARNING: GNU make legacy ::= assignments are not actually skipped, despite the doc comment (line 771) and inline note (line 790) claiming x ::= y is handled.
For a line like VERSION ::= 1.2.3, split_once(':') splits at the first colon, so rest becomes := 1.2.3 (starts with :, not =). The guard therefore does not fire and VERSION is inserted as a spurious make VERSION target. The := form only works because its rest begins with =; the ?= form is skipped only via the no-colon None branch.
Suggested fix (covers ::= whose rest begins with := … without breaking double-colon rules like a:: b: whose rest begins with : …):
| if rest.starts_with('=') { | |
| if rest.starts_with('=') || rest.starts_with(":=") { |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Reviewed by glm-5.2-short · Input: 22.3K · Output: 18.2K · Cached: 389.8K |
2 participants
Summary
Follow-up for #51 review feedback on
qr learnrole-command detection.:=/?=assignments and tabbed recipe bodies; skip Justfile:=/set/export/aliasnext lint(removed in Next.js 16)main.pyis no longer treated as FastAPI / uvicornmake/justbodies when filling Node role commands (nopnpm buildfor a missing package script)cargo run/go run .on a real binary / rootpackage mainpipenv runTest plan
cargo fmt --all -- --checkcargo clippy --all-targets --locked -- -D warningscargo test --lib project_profilefeat/learn-common-commands(feat(learn): common build/test/dev/run/debug commands #51)