Give the build run feature an api module - #83
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
:feature:editordepended on:feature:buildrun's implementation for thirteen symbols. Only one of them,BuildRunApi, was an abstraction; the rest were concrete models the editor reads to render the build console.What changed
New
:feature:buildrun:apiholding the contract and the types that cross it:BuildRunApi, plusBuildExecutionSnapshot,BuildExecutionPhase,InstallExecutionState,StartBuildResultBuildConsoleState,BuildStatus,BuildTaskGroup,BuildTaskLine,BuildLogLine,BuildProblem,BuildArtifact, and thereduceextensionBuildClientMetapreflight:BuildPreflightResult,PreflightWarning,PreflightSeverity,ToolchainVersions, and the checkersRunTargetResolver:feature:editornow depends on the api module.BuildRunCoordinator,BuildInstallRunner,BuildNotifier, the installer and the keep-alive service stay in:feature:buildrun.Editor's tests construct
BuildRunCoordinatorandBuildNotifierdirectly, so:feature:buildrunbecomestestImplementationthere — main depends on the contract, tests exercise the implementation.Boundary baseline is down to a single entry.
Two things the type-closure audit turned up
BuildClientMetawas declared inBuildRunCoordinator.kt, not alongside the interface, even though it is a parameter ofBuildRunApi.start(). Extracting only the obviously-contract files would have left the interface referencing a type still inside the implementation module.RunTargetResolveris a stateless object over:domaintypes with no buildrun coupling, so it sits in the api module rather than behind the interface — the same treatment asgitErrorMessage. Moving it into:domainas a use case is arguably cleaner and remains open.The api module takes a Compose dependency solely for
@Immutableon the console models. That annotation is part of their contract: it tells Compose the state is stable so consumers skip recomposition correctly, and dropping it would silently change recomposition behaviour in the editor.Behavior
No behavior change. Types moved between modules; no logic altered.
Tests
Verification
./gradlew test detekt verifyModuleBoundaries :app:assembleDebug— BUILD SUCCESSFUL. Boundary report: 127 dependencies checked, 1 baseline entry, 0 violations.Verified on an emulator, because the previous api extraction shipped a crash that no JVM test could see:
AbstractMethodError,NoSuchMethodErrororNoClassDefFoundErrorin logcatNote: unlike
GitPanelApi,BuildRunApihas no@Composablemembers, so the default-argument hazard that caused the earlier crash does not apply here.