potiuk commented on PR #43453:
URL: https://github.com/apache/airflow/pull/43453#issuecomment-2456540668

   > @potiuk
   > 
   > > Can you please add a unit test covering it?
   
   
   That was the part of first pass of review. At this stage assesment from me 
is that the change lacks unit test - so it won't be generally accepted anyway.
   
   > We will, but first we want to know if you (== anyone responsible for dbt 
Cloud Operator) are fine with the approach
   
   Maybe. I have no idea, I am not a dbt expert. Possibly someone else who is 
will take a look. But a lot of people won't even look if there is no unit test, 
we generally don't accept PRs where unit tests work before and after the change 
- because it means that the change is not testable and prone to regression. 
   
   Also unit tests help to guide reviewer in understanding what the change does 
- unit tests shows the scenarios in which the change gets used. Often unit test 
explain the reason and circumstances of the change better than words, and as 
such they are often used - especially when changes are small - as the review 
guideline. Also it shows that the author knows what they are doing since they 
are able to reproduce the situation they are talking about in unit tests.
   


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

Reply via email to