Yeah, I've found this to be pretty smooth as well. In most cases comments were already resolved and in lesser cases it was useful to see which conversations still needed addressing before merging. +1 from me!
________________________________ From: Ryan Hatter <ryan.hat...@astronomer.io.INVALID> Sent: Tuesday, January 30, 2024 8:58:30 AM To: dev@airflow.apache.org Subject: RE: [EXTERNAL] [COURRIEL EXTERNE] [ANNOUNCE] Starting experimenting with "Require conversation resolution" setting CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. AVERTISSEMENT: Ce courrier électronique provient d’un expéditeur externe. Ne cliquez sur aucun lien et n’ouvrez aucune pièce jointe si vous ne pouvez pas confirmer l’identité de l’expéditeur et si vous n’êtes pas certain que le contenu ne présente aucun risque. In my experience outside of Airflow, the benefit of not missing a review comment outweighs the friction of being required to resolve each conversation. On Mon, Jan 29, 2024 at 8:47 PM Wei Lee <weilee...@gmail.com> wrote: > I didn't notice much of a difference as a contributor. +1 vote > > Best, > Wei > > > On Jan 30, 2024, at 11:41 AM, Amogh Desai <amoghdesai....@gmail.com> > wrote: > > > > Contrary to my initial expectation of the trouble this would bring in for > > reviewers, it has been > > pretty nice. I have not faced any issues in marking the conversations as > > resolved for the pull > > requests I have reviewed and it has even given me a chance to re review > > prior to approval. > > > > I am happy with this overall and my vote will be a +1 > > > > Thanks & Regards, > > Amogh Desai > > > > On Mon, Jan 29, 2024 at 7:56 PM Aritra Basu <aritrabasu1...@gmail.com> > > wrote: > > > >> I personally haven't had too much friction due to the change and it has > >> helped me keep track of any comments people have made. I remain +1 to > the > >> change so far. > >> > >> -- > >> Regards, > >> Aritra Basu > >> > >> On Mon, Jan 29, 2024, 6:11 PM Jarek Potiuk <ja...@potiuk.com> wrote: > >> > >>> Just wanted to remind everyone, we are nearing the end of the trial > >> period > >>> for "require conversation" feature to be enabled. I have my own > >>> observations and examples, but since I was the one to propose it, I am > >>> likely biased, so I'd love to hear from others what their feedback and > >>> assessment is. Or maybe we need more time to assess it ? > >>> > >>> I would love to hear your thoughts. > >>> > >>> J, > >>> > >>> > >>> On Sat, Dec 30, 2023 at 2:20 PM Jarek Potiuk <ja...@potiuk.com> wrote: > >>> > >>>> After an initial indentation problem in .asf.yaml it's not working as > >>>> expected. So .... let's see how resolving conversations will work for > >> us. > >>>> > >>>> On Sat, Dec 30, 2023 at 12:17 PM Amogh Desai < > amoghdesai....@gmail.com > >>> > >>>> wrote: > >>>> > >>>>> Wooho! Looking to see how this turns out for airflow 😃 > >>>>> > >>>>> On Sat, 30 Dec 2023 at 1:35 PM, Jarek Potiuk <ja...@potiuk.com> > >> wrote: > >>>>> > >>>>>> Hello everyone, > >>>>>> > >>>>>> As discussed in > >>>>>> https://lists.apache.org/thread/cs6mcvpn2lk9w2p4oz43t20z3fg5nl7l I > >>> just > >>>>>> enabled "require conversation resolution" for our main/stable > >>> branches. > >>>>> We > >>>>>> have not used it in the past so it might not work as we think or we > >>>>> might > >>>>>> need to tweak something. > >>>>>> > >>>>>> Generally speaking (if all works) all conversations on PRs should be > >>>>>> resolved before we can merge the PR. This "resolving" is encouraged > >> to > >>>>> be > >>>>>> done by the author when they think the conversation is resolved, but > >>> it > >>>>> can > >>>>>> also be done by reviewers or the maintainer who wants to merge the > >> PR. > >>>>>> > >>>>>> We attempted to describe some basic rules and expectations here: > >>>>>> > >>>>>> > >>>>> > >>> > >> > https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#step-5-pass-pr-review > >>>>>> but undoubtedly there will be questions and issues that we might > >> want > >>> to > >>>>>> solve - so feel free to discuss it here or raise question/issues in > >>>>>> #development channel in slack (I am also happy to be pinged directly > >>>>> about > >>>>>> it and help to resolve any issues/gather feedback). > >>>>>> > >>>>>> J. > >>>>>> > >>>>> > >>>> > >>> > >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org > For additional commands, e-mail: dev-h...@airflow.apache.org > >
