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 >
