mdedetrich commented on PR #1919: URL: https://github.com/apache/pekko/pull/1919#issuecomment-3045393050
> Regarding the configs, they do look a bit strange to me, sharing paths where you would expect them to have their own paths. Changing this is a breaking change and would be better discussed as a standalone issue. We don't seem to get too many complaints so the issue doesn't seem to be affecting people too much in real world scenarios. I would avoid messing with configs in a backwards breaking way if possible. While its still technically allowed under SemVer as its a behavioural change (at least if this is released for 1.2.x) it can still be surprising for end users. I agree this should be discussed and done separately > reordering the code to allow tailrec to work: this does come with risks. All we really have is our test coverage and if that passes then maybe we should be happy enough. But yes, the test coverage could be better. We have a config that controls whether the new code is applied. I wonder if the code path for when the config is set to keep the Pekko 1.0/1.1 (and Akka 2.6) behaviour, that we keep that code free of the tailrec refactor - ie we only try to make the new code path tail recursive and the old code path remains as similar as possible to the code that exists in the main branch prior to this PR. > @Tomassino-ibm I would like the code that tries to replicate the existing behaviour (before this PR) to be as similar as possible to the existing code and the refactors to support the new path only apply to the new path. This doesn't need be 100% - we can discuss having some of the refactor on the old path where it is hard to avoid or causes excessive code duplication. Regarding the tailrec, the intention of writing it as tailrec was actually to maintain the original style so its easier to read and hence make sure that its behaving more correctly. While its true that the original version didn't have `@tailrec`, thats because the implementation was so simple that there wasn't any loops to begin with. I just compared the tailrec version to the older non-tailrec version and the tailrec version is far easier simpler, so my preference would be to stick to the tailrec version (this is why I pushed for it). -- 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
