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

   > > Recently ruff community implemented flake8-type-checking in ruff, and I 
would say that it cover all your concerns.
   > 
   > I hope it become better, previously it required a lot of additional time 
to fix it behaviour.
   > 
   > I we want to enable it into the future and the rules auto-fixable, might 
be better enable `TCH001` and `TCH002` ([list of all 
rules](https://beta.ruff.rs/docs/rules/#flake8-type-checking-tch)), and run 
against airflow repo and create PR, rather than trying to do it manually?
   > 
   > Personally I don't think that we need to have `TCH003` (_Move standard 
library import `{}` into a type-checking block_)
   
   Yes. Ruff rules are great for that. Yes. Agree we do not need TCH003.
   
   I think for cherry-pickability reasons we shoudl connect both - authomatic 
conversion and splitting the "core" changes at least to smalle PRs manually - 
see https://github.com/apache/airflow/pull/33755.  It makes it really hard for 
the release manager to cherry-pick a change that has conflicts when you modify 
a file that has been earlier modified by a PR that changes 180 files in core. 
Basically it means that the release manager will either have to manually 
resolve conflict, or  will have to cherry-pick the refactor that made the 
conflict happen. The former is always prone to errors, the latter is only 
possible if you have 80 localized PRs rather than 1 huge PR changing it.


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