shahar1 commented on PR #33786: URL: https://github.com/apache/airflow/pull/33786#issuecomment-1694495915
> > One that includes the pre-commit Python script and its YAML definition in pre-commit-config.yaml - I excluded files that should be fixed in later PR; see next. > > Another that includes modifications to specific operators, either to align with the requirements immediately or by inserting a FIXME comment to do it later (in case the complexity seems too high to address at the moment). If the reviewers deem it necessary, I can create a separate pull request for this. Regardless, I'll create a new issue for the necessary fixes. > > I think it should be approached differently.. This PR should fail. It should not have exclusioins. It should only be merged when we (or others) fixed all the problems. This is what we do normally. Then this PR open should serve as reminder and incentivisation to fix all the places. So I think in general, the sequence should be reverse: > > 1. add PR that fails and reminds others when they cross the border > 2. make sure (it does not have to be you - you can ping other people to do it) to fix alll the places > 3. continue rebasing this PR seeing less and less problems as you iterate > 4. merge it once all the problems are fixed. > > This is the aproach we've used for other cases that were similar. Sounds good to me :) I removed the exceptions, yet I kept the second commit for inspiration - I'd be happy for feedback regarding the logic that should (not) be in the constructors. -- 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]
