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