[GitHub] [flink] bowenli86 commented on issue #8480: [FLINK-12552][table] Combine HiveCatalog and GenericHiveMetastoreCat…

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

bowenli86 commented on issue #8480: [FLINK-12552][table] Combine HiveCatalog and GenericHiveMetastoreCat…
URL: https://github.com/apache/flink/pull/8480#issuecomment-494188903
 
 
   > Those abstract classes are part of this PR, and they weren't planned as a separate PR. Thus, I don't see a good reason to have a separate JIRA/PR for them.
   
   First of all, I want to clarify that, IMO, adding abstract classes is a good change :) and my suggestion is not to diminish this effort.
   
   But I disagree here because, from what I can tell, it doesn't have much to do with unifying two catalogs, and can happen independently. It doesn't need to be on its own PR. However, I believe it's better for us to create its own JIRA though it's not planned in advance, and have it in its own commit in this PR. This is part of the "Separate Refactoring, Cleanup and Independent Changes" requirement in the [guide](https://docs.google.com/document/d/1JGRVK6udBFa8xbyv7ipPQiLEhxebA64f8KKcaAH0UPY).  The isolation of changes makes review, merge, track much easier.
   
   ------
   
   LGTM besides a few missing parts that I'll take care of for you. Will merge once CI passes.

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


With regards,
Apache Git Services