I'm with Jarek, feel like an adult and like to see which comments are "open" 
and which are "resolved". That give clarity for me when I review where 
something is ready to be merged and which is still in discussion.
But I also do not see a real burden for people disliking it, other than 1-3 
clicks for non-resolved comments.

My vote is +1 keeping it.

Mit freundlichen Grüßen / Best regards

Jens Scheffler

Alliance: Enabler - Tech Lead (XC-AS/EAE-ADA-T)
Robert Bosch GmbH | Hessbruehlstraße 21 | 70565 Stuttgart-Vaihingen | GERMANY | 
http://www.bosch.com/
Tel. +49 711 811-91508 | Mobil +49 160 90417410 | jens.scheff...@de.bosch.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart, HRB 14000;
Aufsichtsratsvorsitzender: Prof. Dr. Stefan Asenkerschbaumer;
Geschäftsführung: Dr. Stefan Hartung, Dr. Christian Fischer, Dr. Markus 
Forschner,
Stefan Grosch, Dr. Markus Heyn, Dr. Tanja Rückert

-----Original Message-----
From: Jarek Potiuk <ja...@potiuk.com>
Sent: Mittwoch, 31. Januar 2024 17:33
To: dev@airflow.apache.org
Subject: Re: [ANNOUNCE] Starting experimenting with "Require conversation 
resolution" setting

I really understand the concerns. I am also not 100% happy either with the way 
you have to unresolve things to see past comments, and how links to old 
resolved comments are difficult to solve. This is certainly a drawback I see, 
and yes - it's a true concern.

And Ash - really my point was not to treat people as non-adults, it's as far 
from it as possible and I think it's not "working" this way either.

The most important benefit I have (personally)  from this is not accidentally 
missing an important comment on a PR that is green, approved, but nevertheless 
there is an important question someone asked and it's been missed accidentally. 
And seeing "unresolved comments' ' before I click merge is the main reason why 
I **reallly** like the feature.

Now I believe the things is what we - as community want to optimize for (I hope 
I correctly gathered the main pro/cons here):

* do we want to optimize for having the comments open and returning to them 
after the PR has been merged and looking at other's comments while the PR is 
open (which is affected)
* or do we want to optimize to not to accidentally miss important stuff at the 
moment when decision is made "let's merge the PR, it's ready and we think all 
the comments are addressed"

I am (as you might realise) somewhat of a merge "fan". I merge a lot of PRs 
(likely 20%-30% of all the PRs) - when I think they are ready. That - of course 
and I would be far from telling that - not necessarily translates to 
"importance" of those PRs and depth and participating in the 
discussions/conversations. Many of my merges are small, insignificant or even 
typo-like, But sometimes I do merge important ones, and for me personally it's 
better to be more certain that I have not missed anything important - it's 
something that saves me, personally,  awfully lot of time and mental energy, 
that otherwise I'd have to lose, multiplied by a number of commits. That's a 
very clear benefit to me, personally. I hope that's clear, and I think those 
who say that it brings no value, I can tell you that in my case it certainly 
does, I hope you will see my perspective.

For others - who mostly take part in discussions, but very rarely click the 
"merge" button - and for those people there are no "merge" benefits, but only 
an increased burden and difficulty increased in other places - when they want 
to see and weigh comments of others for the PR they look at. And that's 
obviously very understandable as well. Trying to see both sides here.

There is no 0/1 answer here, I think. There is always a trade-off - something 
for something. But I think the important thing is that we think what's better 
for the whole project and we weigh both sides (and clearly there are two 
perspectives here - each with their own merits).

Proposal then:

This is a procedural question not a code modification
https://www.apache.org/foundation/voting.html  - so In this case regular voting 
matters and +/-.
I am in Brussels/FOSDEM till next week, so my proposal is to let other comments 
come, and I think it would be great that everyone weighs in and I will throw a 
vote next week. As usual the "disagree but engage" - I am happy with whatever 
outcome it brings, so let's see next week.

In the meantime, If there are other comments, it would be great to hear them 
too.

J.




On Wed, Jan 31, 2024 at 4:55 PM Bolke de Bruin <bdbr...@gmail.com> wrote:

> The point Ash is making is also my point. I am actually faster in
> clicking resolve now, just to move the PR forward in my mind. That
> does not necessarily mean I did a good job at resolving :-).
>
> Bolke.
>
> On Wed, 31 Jan 2024 at 16:26, Elad Kalif <elad...@apache.org> wrote:
>
> > I agree with Ash.
> >
> > I think leaving threads open is a feature not a problem.
> > I used it with referencing todos in new issues and I think it's
> > easier
> when
> > the thread is kept open.
> >
> > Personally if I have a review that is important to me to follow up
> > on
> then
> > I publish it as request changes not as comment. That way if someone
> > wants to override my review he must dismiss it with a note explaining why.
> > That is much more powerful.
> >
> > I am -0 for keeping it.
> >
> > On Wed, Jan 31, 2024 at 1:07 PM Ash Berlin-Taylor <a...@apache.org>
> wrote:
> >
> > > To be clearer about the reason I don't want this
> > >
> > > Often times someone will leave a comment, and I will reply along
> > > the
> > lines
> > > of "yes, fixed in fixup commit x" and want them to see it if they
> > look/come
> > > back, but I don't think it's worth blocking merge on waiting for
> > > them
> to
> > > approve/resolve/re-review.
> > >
> > > But if I resolve the thread it then it makes it invisible/requires
> > > much more active effort on their part to see it.
> > >
> > > Similarly, when reviewing I find I have to expand all the resolved
> > > discussions to see what has already been said otherwise I end up
> > > asking
> > the
> > > same questions ("why this way?" or "what about case Y?")
> > >
> > > If GH let discussions be resolved without also collapsing them I'd
> > > be
> +1,
> > > but mixing the two mens I prefer _not_ resolving discussions.
> > >
> > > -a
> > >
> > > On 31 January 2024 11:00:11 GMT, Ash Berlin-Taylor
> > > <a...@apache.org>
> > wrote:
> > > >I'm a -1 on keeping this as I don't see it gives us any real
> > > >benefit
> > > other than a rubber-stamp. Let's treat people as intelligent grown
> > > ups instead of children who need strict rules.
> > > >
> > > >On 31 January 2024 09:37:50 GMT, Pankaj Koti <
> pankaj.k...@astronomer.io
> > .INVALID>
> > > wrote:
> > > >>+1 to keep this
> > > >>
> > > >>@Bolke de Bruin: I am just thinking more on your point and
> > > >>wondering that if someone has the intent to hide the
> > > >>conversation, they can
> > anyway
> > > >>mark it as resolved irrespective of this configuration, no?
> > >
> >
>
>
> --
>
> --
> Bolke de Bruin
> bdbr...@gmail.com
>

Reply via email to