o-nikolas commented on PR #30727: URL: https://github.com/apache/airflow/pull/30727#issuecomment-1528821699
Hey @potiuk, thanks for the review! 😃 > Just a thought. @o-nikolas . MAYBE there is a way to split that one into two (at least) prs -> one extracting/renaming code/methods, maybe extracting the common k8S executor types, and another moving stuff arround between files and splitting it? The extracted method and modified code that is unrelated to the splitting is a very small amount of code. Most changes are related to the moved code. So I decided to leave it all in one. > This is the technique i used in the "base_job" refactoring case and I think it makes a lot of sense ? Would it be possible that you give it a try ? Yupp, I agree with this philosophy, the overall code I'm delivering is already broken up. By the end it will be about 3 or 4 PRs, with one already merged, this one, and some more to come after. The process becomes very grueling if I separate them all even more, I will end up with 6-8 PRs. Which is especially frustrating when reviews are hard to come by and I must constanty manage the conflicts from main. However, if I have still not convinced you I can try separate out what I can on Monday. Let me know what you think. Thanks again for the review I really do appreciate 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]
