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 > >
