From 670386ed72ee9dc1787ba12a5c8b539a9c1db8d0 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 7 May 2026 14:26:43 -0400 Subject: [PATCH] Address PR feedback --- .claude/skills/remove-config-param/SKILL.md | 2 +- .claude/skills/remove-model-field/SKILL.md | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.claude/skills/remove-config-param/SKILL.md b/.claude/skills/remove-config-param/SKILL.md index 8822aa58e..fecbc6494 100644 --- a/.claude/skills/remove-config-param/SKILL.md +++ b/.claude/skills/remove-config-param/SKILL.md @@ -132,7 +132,7 @@ Follow the full removal steps above. ## Common Gotchas - **Remove all call sites first** — if code still calls `get_config().MY_PARAM` or `settings.MY_PARAM` after the definition is gone, startup or runtime will raise `AttributeError`. -- **Old ConfigRevision rows keep the key** — this is safe; the `Config.__getattr__` lookup simply won't find a registered default and will raise `AttributeError` if code tries to access it. Remove all code references before removing the `ConfigParam`. +- **Old `ConfigRevision` rows retain the key in their JSON blob** — this is harmless and requires no migration. The risk is code: any remaining call to `get_config().MY_PARAM` or `settings.MY_PARAM` after the definition is gone will raise `AttributeError`. Remove all code references *before* removing the `ConfigParam` definition. - **`configuration.py` in user deployments** — removing a static parameter may cause a `TypeError` or silent failure if users have `MY_PARAM = ...` in their local `configuration.py`. Document the removal in the release notes. - **No `ruff format`** on existing files — use `ruff check` only. diff --git a/.claude/skills/remove-model-field/SKILL.md b/.claude/skills/remove-model-field/SKILL.md index 39629f674..1e23c1860 100644 --- a/.claude/skills/remove-model-field/SKILL.md +++ b/.claude/skills/remove-model-field/SKILL.md @@ -155,7 +155,11 @@ If the field was indexed for global search, remove it from the `fields` tuple: 2. If the field was in `clone_fields`, remove it from that tuple. 3. If `clean()` had validation logic specific to this field, remove those clauses. If `clean()` becomes empty, remove the override entirely. 4. For FK fields: remove the `related_name` on the target model is automatic (Django handles it). If the FK was the only reason a related model was imported, remove that import too. -5. For GenericForeignKey fields: if this was the only GFK, also remove the `object_type` ContentType FK and `object_id` integer field, and remove the `models.Index(fields=('object_type', 'object_id'))` from `Meta`. +5. Check `Meta` for references to the field: + - `ordering` — if the field appears in the ordering tuple, remove it (or replace with a remaining field if ordering would otherwise become empty). + - `constraints` — remove any `UniqueConstraint` or `CheckConstraint` whose `fields` list includes this field; if only this field remains, remove the constraint entirely; if other fields remain, remove just this field from the list. + - `indexes` — remove any `models.Index` that includes this field. +6. For GenericForeignKey fields: if this was the only GFK, also remove the `object_type` ContentType FK and `object_id` integer field, and remove the `models.Index(fields=('object_type', 'object_id'))` from `Meta`. ## 11. Generate the Migration @@ -190,7 +194,7 @@ python manage.py migrate | 7 | `tables/.py` | Remove column declaration and from `Meta.fields`, `default_columns` | | 8 | `/ui/panels.py` | Remove attr declaration from panel class | | 9 | `search.py` | Remove from SearchIndex `fields` tuple | -| 10 | `models/.py` | Remove field; clean up `clone_fields`, `clean()`, imports | +| 10 | `models/.py` | Remove field; clean up `clone_fields`, `clean()`, `Meta` ordering/constraints/indexes, imports | | 11 | (user runs) | `makemigrations -n remove__from_ --no-header` then `migrate` | ## Common Gotchas