potiuk commented on PR #63512: URL: https://github.com/apache/airflow/pull/63512#issuecomment-4063878462
I have moved this PR (I closed it accidentally by pushing main to it) to https://github.com/apache/airflow/pull/63672 but let me answer some things here > In an asynchronous open-source environment, 4 hours is a very narrow window. Many of us are balancing this with other work or are in different time zones. If an author pushes a fix and hits an isolated flake or minor linting error, the bot auto-drafts it while they are offline. By the time the requester can fix it, the maintainer (who may only check PRs every few days) has already seen the "Draft" status and moved on. This kills momentum on PRs that were already 99% there. No bots are closing and drafting PRs. Humans will. Drafting a PR is a decision of maintainer, maybe we should indeed change the 4hr to say 24 hrs. But also - drafting PR also is not "closing" it - this is just an information to author "Your PR is not ready - i.e. Draft" + it provides information to the author that they should fix things, with detailed information what they should fix + letting them know that they will have to undraft the PR when it gets green - effectively passing the responsibility to the author to make it green. But you are right we do not explain it well, we should say "take your time feel free to fix things at your own pace" - so that we do not create a sense of urgency. Really - drafting PRs is there because many people just do not check if their PR succeeeded - mainly because it takes time and they do something else. So I treat such Drafting as a signal to them - they will not have to check it, they will get information "hey your PR failed, and you need to fix this and that, but feel free to do it at your pace". Do you think that is a bad thing ? > This kills momentum on PRs that were already 99% there. Not sure - for me getting PR "green" is about maybe 20% in and a lot of people do not even get there Then there is a review, responding to it, iteration, improvements etc. And even if we have different view on the percentage converting PR to draft makes no change here. It's just "make it ready for review" button to click by the author - when they fix all the issues. > By the time the requester can fix it, the maintainer (who may only check PRs every few days) has already seen the "Draft" status and moved on. This is fine. The goal is that reviewers won't even look at PRs that have broken static checks. It's responsibility of author to fix them (i.e. make PR green) before the reviewers look at it. And then also, when the PR is converted to Draft, we clearly inform the author, that they should fix it and convert > If a PR has a maintainer approval or recent comments, could we extend the grace period (e.g., 24-48 hours)? This respects the human review state and the reality that people aren't monitoring CI 24/7. Yes. that is a very good suggestion - actually that solves some of the earlier cases that I saw - we could even change it to 72 HRs when there are comments and maybe even a week when there is approval of maitainer,. The reason why we have this first comment is that I really want to address the situation, that someone opened PR and did not look at it at all, but you are perfectly right that when PR has comments and approvals from maintainers, this grace period should be way longer indeed. > If a PR has passed CI in the past, we could treat new failures as "needs a fix" rather than "needs to be drafted" for a longer period. I think having comments or approval from maintainers is a better idea here, IMHO it's a better indication of readiness for review. There might be incremental changes added to a PR that will make it pass initially and go draft, without seeing any comments or approvals. But of course we can improve it and play with the workflow a bit. > A way for committers to run the review mode locally to see suggestions and logs without the tool auto-drafting or posting to the PR. This is precisely how it works and how it was designed. No comment, no action, will ever be posted without maintainer's deliberate action. Also when commenting, maintainer is able to choose which comment to post or even edit the comment before they get posted. The basic assumption of the tool that it NEVER does anything to a PR without explicit maintainer's approval - the maintainer that sees the suggestion and chooses to post the comments. The comments are NEVER posted by the bot - they are posted by maintainer - on their account and they take responsibility for the comments posted. -- 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]
