Repository navigation
refactor(android): drop the dead scope-level adb server port lane (review follow-ups) - #3390
Conversation
The deleted managed-device reachability was the only producer of a construction-time adb server port, so withAndroidAdbProvider's scoped host transport, the scope serverPort option on createLocalAndroidAdbProvider / createDeviceAdbExecutor and the serial spawner, and the scope checks that guarded them have no caller. A port now reaches adb only through the per-call AndroidAdbExecutorOptions.serverPort, which keeps its coverage. Also rewraps the device-claim rule comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… honors it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…longer reads The production-exports config hides every scripts/ finding through ignoreFindings, so the per-symbol entries for resolveVitestMaxWorkers, readTestScope, ownedTestFiles and the help-conformance *_SAMPLE constants were redundant. The base check:fallow run does not need them either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Size Report
Startup median (7 runs, lower is better):
|
|
This PR is ready. I reviewed e37d01c and found no problems in the code. All 19 checks pass and there are no conflicts. Not blocking: the rewritten doc line near https://github.com/callstack/agent-device/blob/e37d01c/packages/platform-android/src/adb-transport.ts#L532 runs past the wrap width the rest of that comment block uses, so you can rewrap it or leave it. Two limits on my review. I did not run check:fallow locally to confirm the base .fallowrc.json no longer needs the removed scripts/ entries, so I relied on the green checks for that. I also did not check whether parseAndroidAdbArgv could throw on argv after the -s pair in the old no-port override path, which the new path no longer parses. Production callers build argv from deviceShellArgv, so I judged that negligible. Nothing else stands between this PR and merge. |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Review follow-ups from #3370 and #3372 that missed the stack merge:
managed-device-reachability.ts).withAndroidAdbProvider's scoped host transport and the construction-timeserverPortoncreateLocalAndroidAdbProvider,createDeviceAdbExecutor, the serial executor and the spawner are removed, with their scope-only tests. A port now reaches adb only through the per-callAndroidAdbExecutorOptions.serverPort, which keeps its coverage.androidAdbSerialTarget(an internal mechanics facet) loses its port parameter, and port reverse, which has no per-call port, can no longer target a private server; nothing produced that.agent-device/android-adbfunctions run the caller's own executor, so the per-callserverPorttakes effect only when that executor honors it.scripts/ignoreExportsentries are dropped, since the production config'signoreFindings: ["scripts/**"]already covers them (chore(deps): upgrade fallow to 3.32 #3370 review). The basecheck:fallowdoesn't need them either.device-claim-rule.tsdoc comment.9 files, +131/−692.
Validation
Tested
e37d01ca7on mainc2c09d476:pnpm check:affected --runpassed (all runnable checks). Android: 115 files / 1,017 tests.check:fallowandcheck:production-exportspass on fallow 3.32. Dead-path removal, so no live device run applies.🤖 Generated with Claude Code