He-Pin commented on PR #746: URL: https://github.com/apache/pekko-grpc/pull/746#issuecomment-4788875674
Thanks for the clarifications ā you're right on the main points: 1. **PB.recompile block removal**: Fair point about the scripted tests ā if source discovery were broken, they'd catch it. I also misread the original issue: the problem was proto resources not being included in the JAR, not class generation failure. Withdrawing the š“ concern. 2. **O(nĀ²)**: Thanks for the fix. 3. **Parameter name**: Fair enough, `string` does convey the type. Minor nit, no need to change on its own. The approach of using `packageBin / mappings` for explicit control over JAR contents is the right sbt idiom ā more deterministic than relying on unmanaged resource directory scanning timing. One remaining minor note: the scripted test assertion using `.mkString.contains(...)` could be made more robust with `.exists(_.getPath.endsWith(...))` for clearer failure messages, but that's a nit. Overall the direction looks good. Looking forward to the final version once the `google-cloud-pub-sub-grpc` case is sorted out. -- 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]
