He-Pin commented on PR #3432:
URL: https://github.com/apache/pekko/pull/3432#issuecomment-5301516954

   ## Review Notes
   
   Thanks for putting this together! A few issues I noticed:
   
   ### Must fix
   
   1. **`diff.txt` should be removed** — this file shouldn't be committed to 
the repository.
   
   2. **`split-brain-resolver.md` missing one fix** — line 384: "links but 
they" should be "links, but they" (comma after "links"). This was part of the 
upstream akka/akka-core#32019 fix.
   
   ### Missing from v2.8.3...v2.8.4 range
   
   3. **Retry short-circuit Java test** (akka/akka-core#32035) — Pekko already 
has the `shouldRetry: (T, Throwable) => Boolean` API (since 1.1.0), but lacks a 
Java DSL directional test for `Patterns.retry` with `Predicate2`. See 
apache/pekko#3435.
   
   4. **Harden PersistentActorRecoveryTimeoutSpec** (akka/akka-core#32030) — 
The "should not interfere with receive timeouts" test can be hardened to verify 
that a short receive timeout doesn't trigger recovery timeout, and that 
recovery timeout still fires when journal is unresponsive. See 
apache/pekko#3436.
   
   ### Minor observation
   
   The `InmemJournal` uses `scala.jdk.DurationConverters._` with `.toScala` — 
this is the correct approach for Scala 3 cross-compilation (vs 
`pekko.util.JavaDurationConverters._` which fails under `-Xfatal-warnings` in 
Scala 3).


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to