[GitHub] [beam] piotr-szuberski commented on pull request #12423: [BEAM-10135][BEAM-10136] Refactor jdbc external transform registrar

[GitBox <gi...@apache.org>]

piotr-szuberski commented on pull request #12423:
URL: https://github.com/apache/beam/pull/12423#issuecomment-667916022


   > "During writing kinesis for cross-language it was suggested to emplace all classes connected to cross-language in one file"
   > 
   > I think Java generally recommends keeping classes in separate files, doesn't it ? I think either way should be fine as long as x-lang framework is concerned.
   
   @chamikaramj In general yes, but most of beam connectors have huge files with nested classes instead of splitting them to separate packages and small files. In the case of external transforms it's a bit more elegant because ExternalTransformRegistrar has to override function knownTransforms() which assumes there can be more than one transform under it. The builders actually should also be in separate files from the registrar.
   
   But I agree that it's not that necessary to put everything into one file, so if you prefer it to be like it is now then I'll close this PR.


----------------------------------------------------------------
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.

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