bvolpato commented on PR #27534: URL: https://github.com/apache/beam/pull/27534#issuecomment-1643833955
Wow! That's awesome, great improvements. Nice work. I don't have any red flags, just slightly concerned if we have good test coverage around this change. It seems that we don't have unit tests for any of `FromRowUsingCreator` / `GetterBasedSchemaProvider`, but we rely on `GetterBasedSchemaProvider`'s subclasses to be tested (`AutoValueSchemaTest`, `AvroSchemaTest`, `ThriftSchemaTest`, `JavaFieldSchemaTest`, `JavaBeanSchemaTest`, `ProtoMessageSchemaTest`). I think it should be fine, but I don't know if we cover all scenarios (I'll try to look at some of those tests). @chamikaramj Can you please take a look too? -- 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]
