aglinxinyuan commented on PR #7785: URL: https://github.com/apache/texera/pull/7785#issuecomment-5460814113
Both must-fix items are addressed — no code changed for either, so the head is still `642a6dc`. **Title.** Now `refactor(amber): remove dead members and unreachable guards` (59 chars). I widened your noun from "helpers" to "members" so it also covers the `IgnoreReply` constant and the write-only `lastCompletedLogicalPlan` var, not just the two `PekkoActorService` methods — the type and the length are as you asked, and AGENTS.md's ceiling is ~60 rather than 75, so it fits either way. The old title's "four unreachable guards" was itself a miscount (three guards plus a var); dropping the enumeration settles that too. **The `-1` claim.** You're right and the sentence is gone. Literal `-1` commandIds are built at `main_loop.py:752`, `start_worker_handler.py:71` and `:91`, and `backpressure_handler.py:57`. The description now states what is actually true, which is also the stronger argument: Python never referenced the Scala constant, and neither language compares against the *value* — both gate the reply on the sign, `noReplyNeeded(id) = id < 0` at `AsyncRPCServer.scala:113` and `_no_reply_needed(command_id) = command_id < 0` at `async_rpc_server.py:153`. So removing the unreferenced name changes no dispatch decision on either side. Your trace matches what I re-verified. The rebase from your earlier comment is done — the only conflict was one scaladoc line in `WorkflowServiceSpec` that #7754 had renamed (`resolveWarehouseName` → `resolveLakekeeperWarehouseName`), sitting directly under the bullet this PR deletes; both edits are kept. The patch is otherwise byte-identical to what you reviewed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
