Make run-state courses optional in dev data and forward make flags#2790
Make run-state courses optional in dev data and forward make flags#2790kernicPanel wants to merge 2 commits intobatch-orders-v2from
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes the creation of “run-state” demo courses opt-in to keep default dev data generation lighter, and updates the make dev-data target to forward trailing CLI flags to the underlying Django management command.
Changes:
- Add
--run-state-coursesflag tocreate_dev_dataand gate run-state course creation behind it. - Update
Makefileto forward extra tokens passed tomake dev-datatocreate_dev_data.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/richie/apps/demo/management/commands/create_dev_data.py |
Adds an opt-in CLI flag and conditional block for creating run-state courses. |
Makefile |
Introduces argument forwarding logic for make dev-data to pass flags through to create_dev_data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ifeq (dev-data,$(firstword $(MAKECMDGOALS))) | ||
| DEV_DATA_ARGS := $(wordlist 2,$(words $(MAKECMDGOALS)),$(MAKECMDGOALS)) | ||
| $(eval $(DEV_DATA_ARGS):;@:) | ||
| endif | ||
|
|
||
| dev-data: ## create dev data if app container is running | ||
| @echo "Check app container is running..." | ||
| @if [ $(shell docker container inspect -f '{{.State.Running}}' "$(shell $(COMPOSE) ps -q app)") = "false" ] ; then\ | ||
| echo "❌ App must be up and running to create dev data.";\ | ||
| exit 1;\ | ||
| fi | ||
| @$(MANAGE) flush --no-input | ||
| @$(COMPOSE_EXEC_APP) python sandbox/manage.py create_dev_data ${ARGS} | ||
| @$(COMPOSE_EXEC_APP) python sandbox/manage.py create_dev_data $(DEV_DATA_ARGS) | ||
| @${MAKE} search-index |
There was a problem hiding this comment.
make dev-data -- --run-state-courses will result in DEV_DATA_ARGS containing the literal --, so the invoked command becomes create_dev_data -- --run-state-courses. In argparse, -- ends option parsing, which means --run-state-courses will be treated as a positional and the command will fail with an unrecognized argument error. Filter -- out of the forwarded arguments (while still creating a dummy rule for it so make doesn’t try to build it) before passing args to create_dev_data.
| "--run-state-courses", | ||
| action="store_true", | ||
| default=False, | ||
| help="Create courses with explicit run states for ordering testing.", |
There was a problem hiding this comment.
Help text is grammatically awkward: "for ordering testing". Consider rephrasing to something like "to test ordering" or "for ordering tests" for clarity.
| help="Create courses with explicit run states for ordering testing.", | |
| help="Create courses with explicit run states to test ordering.", |
Make the creation of courses with explicit run states optional to keep the default dev data generation lightweight and avoid polluting the catalog with test courses unless needed.
Using the GNU make `--` convention stops make from parsing trailing
flags as its own options, letting us write:
make dev-data -- --run-state-courses --credential
instead of the awkward ARGS="..." form.
kernicPanel
force-pushed
the
dev-data-run-state-courses-option
branch
from
cf606f4 to
5764078
Compare
Purpose
Make the creation of courses with explicit run states optional in
make dev-datato keep the default dev data generation lightweight.Proposal
--run-state-coursesflag tocreate_dev_dataso that thegeneration of the 24 test courses with explicit run states is opt-in
make dev-datato the underlying commandusing the GNU make
--convention, allowingmake dev-data -- --run-state-coursesinstead of the awkwardmake dev-data ARGS="..."form