Conversation
📝 WalkthroughWalkthroughIntroduces a modular include-based Makefile and multiple new make fragments, removes the old flat makefile, adds containerized make targets, updates Dockerfile to multi-stage dev/prod with non-root user, restructures README and docs, and tweaks editor, pre-commit, and ruff settings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/PROJECT_DESCRIPTION.md`:
- Around line 343-354: Pre-commit YAML example has incorrect indentation for the
hooks list item; update the snippet in PROJECT_DESCRIPTION.md so the "- id:
check-migrations" entry is nested under "hooks:" (i.e., indent the list item to
align as a child of hooks:), ensure the "repo:", "rev:", and "hooks:" keys
remain left-aligned as shown and the dash for the hook list is indented to be
under hooks: in the .pre-commit-config.yaml example.
In `@make/common.mk`:
- Around line 31-39: The uv-add and uv-add-dev targets call $(call require,pkg)
but no require function is defined; add a small Makefile function named require
that checks a variable is non-empty and prints an error/returns non-zero if
missing (so calls like $(call require,pkg) validate pkg), or alternatively
remove the $(call require,pkg) invocations and inline a conditional check in the
uv-add and uv-add-dev targets to error out when $(pkg) is empty; reference the
existing targets uv-add and uv-add-dev and implement the require function (or
replace its usage) to ensure the pkg parameter is validated before running uv
add and $(MAKE) build.
In `@README.md`:
- Around line 44-69: The README contains an incomplete sentence "In the project
root, create and configure the following files." in the Setup section; either
remove that sentence entirely or replace it with a concrete list of files and
brief descriptions (e.g., .env, docker-compose.yml, config/*.json) so readers
know what to create/configure—look for the exact sentence text to update in the
README's Setup section and implement one of those two fixes.
🧹 Nitpick comments (1)
make/common.mk (1)
1-3: Consider adding.PHONYdeclaration for all targets.All targets in this file are phony (they don't create files). Adding a
.PHONYdeclaration prevents unexpected behavior if a file with the same name as a target is ever created.♻️ Suggested addition at the top of the file
+.PHONY: bash build check check-all down format run shell test uv-add uv-add-dev uv-upgrade + DC = docker compose -f compose.yaml RUN = $(DC) run --rm app RUN_IT = $(DC) run --rm -it app
6888d15 to
064eb9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 65: The README contains a broken link to "#configuration"; update that
link so it points to the existing configuration section or add a local heading:
either change the anchor to "docs/PROJECT_DESCRIPTION.md#configuration" wherever
the `.env` edit instruction references the Configuration section, or add a
top-level "## Configuration" heading in README (with the same content or a short
pointer to docs/PROJECT_DESCRIPTION.md) so the existing `#configuration` link
resolves correctly.
064eb9e to
b640887
Compare
|



Closes MPT-17674