thejens commented on pull request #15367:
URL: https://github.com/apache/airflow/pull/15367#issuecomment-822405882


   @marcosmarxm 
   Before I change more; I believe these are the three outstanding items from 
your review. How strongly do you feel about these? As in - before I go ahead 
and modify I'm keen to present some "why's" for my decisions...
   
   How important would you say it is to move functionality into a hook? Solving 
the use-case was easy using two already existing hooks and I don't think I need 
to create an additional hook given I don't implement any new functionality in 
how we talk with BigQuery
   
   Further I think mutating a mutable object shouldn't be a problem in this 
case and I am not too keen to deepcopy it for the sake of it.
   
   Lastly the naming of the schema_fields parameter, I wanted to be consistent 
with naming and input format of that field between this and other operators, do 
you think it's a problem? Changing it into updates_to_schema_fields or similar 
is an easy possibility.
   
   


-- 
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:
[email protected]


Reply via email to