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 >
