This reflect my feelings as well. I'm not convinced we are solving something that needs to be solved.
B. Sent from my iPhone > On 19 Dec 2023, at 21:05, Ash Berlin-Taylor <a...@apache.org> wrote: > > Weak -1 from me, because I don't think this needs to be enforced/required > part of the workflow. > > I.e. Convention over enforcement and treating people as mature adults not > children who need guard rails. > > -ash > >> On 19 December 2023 13:12:05 GMT, Jarek Potiuk <ja...@potiuk.com> wrote: >> Hey everyone, >> >> TL;DR; I have a small proposal/discussion proposal to modify a bit the >> branch protection rules for Airflow. Why don't we add a protection >> rule in our PRs that requires all the comments in the PRs to be >> "marked as resolved" before merging the PR ? >> >> I have been following myself - for quite some time - an approach that >> whenever there are comments/suggestions/doubts in my PRs I do not >> merge the PR until I **think** all of those have been addressed >> (somehow). >> >> The resolution might not be what the person commenting wants directly, >> it might be "I hear your comment, and there are good reasons to do >> otherwise" or simply saying - "I know it could be done this way but I >> think otherwise" etc. etc. But sometimes I miss that there is a >> comment that I have not reacted to, I skipped it unconsciously etc. >> >> I think having "some" kind of reaction to comments and deliberate "I >> believe the conversation is resolved" is a very good thing and having >> the author making a deliberate effort to "mark the conversation as >> resolved" is a sign it's been read, though about and consciously >> reacted to. >> >> I've learned recently that you can add protection rule that will >> require all conversations on PR to be resolved before merging it, I >> even went to a great length to create (and get merged) a PR to ASF >> infra to enable it via .asf.yml feature >> (https://github.com/apache/infrastructure-p6/pull/1740) - so we can >> enable it now by a simple PR to our .asf.yaml enabling it. >> >> I'd love to try it - but of course it will have to change a bit the >> workflow of everyone, where the author (or reviewer, or maintainer) >> will have to mark all conversations as resolved deliberately before >> merging PR. >> >> I'd love to enable it - at least to try and see how it can work - but >> I understand it might add a bit of burden for everyone, however, I >> think it might be worth it. >> >> WDYT? >> >> J. >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org >> For additional commands, e-mail: dev-h...@airflow.apache.org >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org For additional commands, e-mail: dev-h...@airflow.apache.org
