ptrdom commented on code in PR #194:
URL: 
https://github.com/apache/pekko-persistence-r2dbc/pull/194#discussion_r1994280618


##########
projection/src/test/scala/org/apache/pekko/projection/r2dbc/TestConfig.scala:
##########
@@ -71,6 +71,9 @@ object TestConfig {
       }
     }
     pekko.actor.testkit.typed.default-timeout = 10s
-    """)))
+    """))
   }
+
+  // FIXME ideally every dependant that combines this config with other 
configs should load/resolve at their callsites

Review Comment:
   Idea is that configs should first be merged, then resolved. Current `main` 
branch code resolves `TestConfig`, then uses it as fallback in test overrides, 
and if test needs to override any substitutions it is unable, because they were 
already resolved.
   
   It felt weird to me to create an issue for it because it is kind of a test 
hygiene thing that only maintainers might somewhat care about, but I wanted to 
at least mention it. Do you think this is a valid improvement that warrants an 
issue or should this comment just be removed?
   
   > Dependent is the correct spelling.
   
   IIUC this is a UK/US English thing, but I can of course fix.



-- 
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: notifications-unsubscr...@pekko.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For additional commands, e-mail: notifications-h...@pekko.apache.org

Reply via email to