feat(koin): add koin module (runtime DI deps + frameworkModule) - #9
feat(koin): add koin module (runtime DI deps + frameworkModule)#9Ziedelth wants to merge 1 commit into
Conversation
Ziedelth
left a comment
Ziedelth
left a comment
There was a problem hiding this comment.
Review Hermes — PR #9 (fan-out 3 reviewers neutres : mon modèle + gpt-5.6-sol high + kimi-k3 max, sans contexte de conversation)
Verdict : Commentaire — 3 points sûrs, dont 1 conforme à une guideline explicite (TESTING §6). ✅ :koin:build OK, api() conforme « Dependency Sharing », style/KDoc/English OK.
🟠 Guideline (vérifiée)
- koin/src/main/kotlin/KoinModule.kt:12 — API publique
frameworkModule()sans test → violeTESTING.md§6 (« a public API change without a corresponding test is an incomplete change »). Ajouter un test minimal (charger le module via Koin et vérifier le démarrage).
🔵 Qualité (vérifié)
- gradle/libs.versions.toml:46 (+ :15) —
koinGradlePluginet sa versionkoinCompilersont mortes : aucune référence dans le repo (scan complet) — vestige du plugin abandonné. Les retirer. - koin/src/main/kotlin/KoinModule.kt:12-13 —
module { }vide : le KDoc annonce des définitions qui n'existent pas. Soit retirerframeworkModule()tant qu'il n'y a rien à enregistrer (le module reste le porteur des deps viaapi(...)), soit y ajouter la première définition réelle.
⚠️ Points incertains / à trancher ensemble
- Bundle
koinEcosystem? : le repo groupe ses deps en bundles (ktorServerEcosystem…) ; ici 3api(...)individuels. Non écrit dans les guidelines → [UNCERTAIN], à voir avec toi. - À quoi sert le module sans définitions ? : le sous-module
koinn'est pour l'instant qu'un porteur de dépendances (koin-core/koin-annotationsviaapi) + unframeworkModule()vide. Est-ce le rôle voulu, ou doit-il porter de vraies définitions framework ? - Worker (PR #21) : sa CI passera après la publication du module
koin(merge framework + push master) — dépendance d'ordonnancement.
Révisé par Hermes (fan-out 3 modèles neutres).
| * Provides a single place to register framework-wide definitions. Consumer services can compose it | ||
| * with their own modules when starting Koin. | ||
| */ | ||
| fun frameworkModule(): Module = |
There was a problem hiding this comment.
🟠 Guideline (TESTING.md §6) : frameworkModule() est une API publique du framework sans test (« a public API change without a corresponding test is an incomplete change »). Ajouter un test minimal : démarrer Koin avec frameworkModule() et vérifier le chargement.
| * with their own modules when starting Koin. | ||
| */ | ||
| fun frameworkModule(): Module = | ||
| module { } |
There was a problem hiding this comment.
🔵 Qualité : module { } est vide — le KDoc annonce un « single place to register framework-wide definitions » mais aucune n'existe. Retirer frameworkModule() tant qu'il n'y a pas de définition réelle à enregistrer (le module reste le porteur des deps via api(...)), ou y ajouter la première définition.
| koinBom = { module = "io.insert-koin:koin-bom", version.ref = "koin" } | ||
| koinCore = { module = "io.insert-koin:koin-core" } | ||
| koinAnnotations = { module = "io.insert-koin:koin-annotations" } | ||
| koinGradlePlugin = { module = "io.insert-koin:koin-gradle-plugin", version.ref = "koinCompiler" } |
There was a problem hiding this comment.
🔵 Qualité (vérifié) : koinGradlePlugin (+ la version koinCompiler ligne 15) sont inutilisés dans tout le repo (scan complet) — vestige du wrapper abandonné. Les retirer du catalog.
Ziedelth
left a comment
Ziedelth
left a comment
There was a problem hiding this comment.
Review Hermes (complément) — reviewer principal (fan-out neutre)
Vérifié dans le repo : 4 points supplémentaires au premier tour de review.
Détail
koin/AGENTS.mdmanquant : tous les modules du framework (core,cache,exposed,validator,ktor) ont unAGENTS.mddédié (règle « Submodule Rules » du rootAGENTS.md) ; le nouveau modulekoinn'en a pas — incohérence de structure.guidelines/ARCHITECTURE.mdnon mis à jour : la liste « Module Responsibilities » + le diagramme de dépendances n'incluent paskoin.settings.gradle.ktssans newline terminale : pré-existant sur master, mais la PR touche exactement ces lignes.koinGradlePlugin= config cassée :version.ref = "koinCompiler"(1.0.2) pointe versio.insert-koin:koin-gradle-plugin:1.0.2qui n'existe pas (404 — les versions de cet artefact suivent la ligne de koin : 4.2.2 ;1.0.2est une version dekoin-ksp-compiler). Une entrée de plugin Gradle va dans[plugins], pas[libraries].
| @@ -0,0 +1,9 @@ | |||
| plugins { | |||
There was a problem hiding this comment.
🔵 Structure (vérifié) : tous les modules du framework (core, cache, exposed, validator, ktor) ont un AGENTS.md dédié (règle « Submodule Rules » du root AGENTS.md) — le nouveau module koin n'en a pas. Ajouter un koin/AGENTS.md bref (portée du module + conventions Koin). Idem : guidelines/ARCHITECTURE.md (liste « Module Responsibilities » + diagramme) ne mentionne pas koin.
| ":ktor", | ||
| ":cache" | ||
| ":cache", | ||
| ":koin" |
There was a problem hiding this comment.
🔵 Nit (vérifié) : fichier sans newline terminale (pré-existant sur master, mais la PR modifie exactement ces lignes) — en profiter pour terminer par un \n.
| junitJupiter = "6.1.2" | ||
| mockk = "1.14.11" | ||
| koin = "4.2.2" | ||
| koinCompiler = "1.0.2" |
There was a problem hiding this comment.
🔵 Config cassée (vérifié) : koinGradlePlugin avec version.ref = "koinCompiler" (1.0.2) pointe vers io.insert-koin:koin-gradle-plugin:1.0.2 qui n'existe pas (404 ; les versions de cet artefact suivent koin : 4.2.2 ; 1.0.2 est une version de koin-ksp-compiler). De plus une entrée de plugin Gradle va dans [plugins], pas [libraries]. Supprimer les deux entrées tant qu'aucun consommateur n'en a besoin (ou les déclarer correctement dans [plugins]).
1 participant
Ajoute un sous-module⚠️ Le plugin binaire testé pour cacher le Koin compiler plugin a été retiré : la distribution du plugin
koinau framework :\n- exposekoin-bom/koin-core/koin-annotationsenapi()(versions centralisées dans le catalog) ;\n- fournit unframeworkModule()de base pour démarrer Koin.\n\nPublie vers GitHub Packages (convention maven-publish existante).\n\nio.insert-koin.compiler.plugin(version séparée 1.0.2) et l'application runtime d'un plugin par un autre plugin Gradle se sont révélées non fiables/non vérifiables — voir discussion.