build: declare django-simple-history and layer openedx_catalog - #2
build: declare django-simple-history and layer openedx_catalog#2jesperhodge wants to merge 3 commits into
Conversation
3a83c40 to
7fffe40
Compare
5a61032 to
27e1c55
Compare
django-simple-history is pinned in the compiled requirements only as a transitive dependency of edx-organizations, and simple_history is absent from INSTALLED_APPS, so HistoricalRecords() would not work. Declare it directly and register the app in both settings modules. HistoricalRecords() itself works without the app installed, but its admin integration and management commands do not, and the package ships no system check to say so. openedx_catalog appears in neither .importlinter's root_packages nor its layering contract, so the first openedx_learning to openedx_catalog import would pass unexamined. The criteria models scope to a CourseRun, so that import is about to exist. Refs openedx#641 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Issue openedx#641's AC calls for openedx_content | openedx_catalog so the layers contract doesn't also decide the catalog-to-content direction, which src/openedx_catalog/ARCHITECTURE.md still records as TBD.
6ed0b17 to
be9f59b
Compare
mgwozdz-unicon
left a comment
mgwozdz-unicon
left a comment
There was a problem hiding this comment.
From Claude:
-
.importlinter'slayerscontract droppedopenedx_contentfrom thelayerslist without replacing it.5de99d4removed theopenedx_contentline and left only a comment in its place, and it never landed back on its own line, not even inside18c61c3'sopenedx_content | openedx_catalogsibling entry, which then got trimmed away entirely inbe9f59b. Right nowopenedx_contentdoesn't appear anywhere inlayers, solint-importsno longer enforces the rule that predates this PR: nothing belowopenedx_learningmay import it. Please putopenedx_contentback into thelayerslist. -
Separate question:
openedx_catalogwas added toroot_packagesby this PR but also never landed inlayers, it was added asopenedx_content | openedx_catalogin18c61c3and removed again inbe9f59b. Was dropping it intentional? Your own commit message on5de99d4says the criteria models scope to aCourseRun, so anopenedx_learningtoopenedx_catalogimport is coming, and without an entry inlayers, the linter won't catch it when it lands.
2 participants
What this does
Refs openedx#641