Conversation
Conditionally append MARIMO_FOLDER to DEPTRY_FOLDERS and add --ignore DEP004 to DEPTRY_IGNORE when the directory exists. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create src.mk to conditionally append SOURCE_FOLDER to DEPTRY_FOLDERS when the directory exists. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Run deptry on all folders in DEPTRY_FOLDERS with DEPTRY_IGNORE flags. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The deptry target is now defined in src.mk with configurable DEPTRY_FOLDERS and DEPTRY_IGNORE variables. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The deptry target was moved to src.mk but the test fixture wasn't updated to include this file, causing three test failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the deptry dependency checking functionality from a monolithic target in .rhiza/rhiza.mk into a modular architecture split across multiple files. The refactoring enables conditional inclusion of both SOURCE_FOLDER and MARIMO_FOLDER in deptry checks, with appropriate flags applied automatically based on which directories exist.
Changes:
- Extracts deptry target from rhiza.mk into a new modular file src.mk
- Adds conditional MARIMO_FOLDER support to deptry configuration via 03-marimo.mk
- Updates test suite to recognize the new src.mk file
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.rhiza/rhiza.mk |
Removes the old monolithic deptry target definition (lines 233-245) |
.rhiza/make.d/src.mk |
New file defining the deptry target with conditional SOURCE_FOLDER inclusion and variable-based folder configuration |
.rhiza/make.d/03-marimo.mk |
Adds conditional logic to append MARIMO_FOLDER to DEPTRY_FOLDERS and add --ignore DEP004 flag when the directory exists |
tests/test_rhiza/test_makefile.py |
Updates SPLIT_MAKEFILES list to include the new src.mk file |
| ## Makefile.src - Source folder configuration | ||
| # This file is included by the main Makefile | ||
|
|
||
| # Add SOURCE_FOLDER to deptry configuration if it exists | ||
| ifneq ($(wildcard $(SOURCE_FOLDER)),) | ||
| DEPTRY_FOLDERS += $(SOURCE_FOLDER) | ||
| endif |
There was a problem hiding this comment.
Consider initializing DEPTRY_FOLDERS and DEPTRY_IGNORE variables at the top of this file before appending to them, for clarity and consistency with Make best practices. While using += on undefined variables works in Make (treating them as initially empty), explicit initialization makes the code more maintainable and self-documenting.
For example, add at the top:
DEPTRY_FOLDERS ?=
DEPTRY_IGNORE ?=
This is especially important since this file defines the deptry target and should be the authoritative source for these variables.
| ##@ Quality and Formatting | ||
| deptry: install-uv ## Run deptry |
There was a problem hiding this comment.
For consistency with other .mk files in this directory (e.g., 01-test.mk, 03-marimo.mk), consider adding a .PHONY declaration for the deptry target at the top of this file after the variable definitions:
.PHONY: deptry
While the target is already declared as PHONY in the main rhiza.mk file, having it declared locally makes the file more self-contained and follows the pattern established in other make.d files.
| ## Makefile.src - Source folder configuration | ||
| # This file is included by the main Makefile |
There was a problem hiding this comment.
Consider renaming this file to follow the numeric prefix convention documented in .rhiza/make.d/README.md. Since this file defines a target (deptry) which is a quality checking tool, it could be numbered in the 20-79 range (e.g., "02-src.mk" or similar) for consistency with other target-defining files like 01-test.mk, 03-marimo.mk, etc.
However, if the intent is for this file to represent core/fundamental source folder configuration (without a prefix to indicate its foundational nature), then the current naming is acceptable.
| @printf "${BLUE}[INFO] DEPTRY_FOLDERS: ${DEPTRY_FOLDERS}${RESET}\n" | ||
| @if [ -n "$(strip ${DEPTRY_FOLDERS})" ]; then \ | ||
| printf "${BLUE}[INFO] Running: $(UVX_BIN) -p ${PYTHON_VERSION} deptry ${DEPTRY_FOLDERS} ${DEPTRY_IGNORE}${RESET}\n"; \ |
There was a problem hiding this comment.
The INFO output on lines 11-13 is more verbose than the pattern used in other targets (see 01-test.mk, 02-book.mk, etc.), which typically just print a brief description like "Running deptry..." rather than showing variable values and the full command.
Consider simplifying to match the existing pattern:
@printf "${BLUE}[INFO] Running deptry on ${DEPTRY_FOLDERS}...${RESET}\n"
However, the current verbose output could be valuable for debugging during the initial rollout of this refactored functionality, so this is a minor style preference rather than a requirement.
Conditionally append MARIMO_FOLDER to DEPTRY_FOLDERS and add --ignore DEP004 to DEPTRY_IGNORE when the directory exists.