Scope feature dependencies on data modules to real usage - #76
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
An audit of the module graph found that most feature-to-data Gradle edges are not real. Of 18 declared
implementation(projects.data.*)edges across the seven feature modules, only three are backed by main-source usage. The rest are leftovers that make the dependency graph look far more tangled than the code actually is, slow builds down by over-linking, and would otherwise have to be encoded as "existing violations" when dependency-rule enforcement is added.Removing them first means the enforcement baseline describes real coupling rather than accumulated noise.
What changed
Build files only. No production or test source is touched.
testImplementation, since they are used only from test sources: projects→templates (TemplateRegistryTest), editor→local (EditorRootInvalidationTest,EditorTabLifecycleTest).core:commonand buildrun→designsystem. Buildrun contains no@Composableat all, so it never needed the design system.Kept, because they are genuinely used from main source:
data:ai—AiProviderCatalogdata:build—GradleProjectReader,ActiveBuild,ActiveBuildRepositorydata:build—GradleProjectReaderBehavior
No behavior change. Nothing is added to or removed from the app's runtime classpath:
:appdeclares all five data modules directly, and every Koin definition lives inapp/di, so feature-to-data edges were never what put these classes in the APK. None of the removed modules contributes a manifest or anysrc/main/res, so manifest merging and resource resolution are unaffected.On why a green build is not by itself proof
implementationdependencies are non-transitive at compile time, so removing one that was genuinely unused cannot break compilation — a passing build is close to tautological here. The removals were justified by symbol-level usage analysis instead, and separately checked for runtime reliance: Koin resolution by type, reflection,Class.forName, service loaders, manifest components, and resource lookup. No removed edge is required by any of those.The split package that nearly caused a wrong deletion
:data:gitand:data:localboth declare the Kotlin package rootcom.ahmadkharfan.androidstudiolite.data.local. A package-based search therefore cannot tell which module owns a symbol, and initially suggestededitor → data:gitwas needed by tests. Resolving it by locating each declaring file showed the two symbols involved —FileChangeBusandLocalFileContentRepository— are both declared in:data:local. Soeditor → data:gitis unused andeditor → data:localis test-only.Note the reverse edge
:data:git → :data:localis legitimate and untouched:JGitGitRepositoryreally does useFileChangeBus.Scope
:domaincontracts for them (GradleProjectReader,ActiveBuildRepository,AiProviderCatalog) is a follow-up, kept separate because it touches production code and DI wiring rather than only build files.data.localsplit package itself is not fixed here; that is its own change.Tests
No test changes. The existing suites are the check: if any deleted edge had really been used, compilation would fail immediately, and the two downgraded to
testImplementationwould break their test source sets.Verification
Verified from a clean worktree at the branch tip, so no unrelated local work could mask a failure:
./gradlew test detekt :app:assembleDebug— BUILD SUCCESSFULUsage was additionally confirmed per symbol rather than per package, to avoid the split-package trap described above.