I don't want you to write an essay; I want to point out, that there is a third state, besides block and approve. I don't want to never have that code in case, the author disagrees with me or I am missing something.

Teams I have worked in often have an Approved subject to comments” with th “all comments must be resolved” marker set.

This somewhat violates the articles “approval or not is the only bit that matters” argument, but it at least forces participants to at least acknowledge an issue, and either provide a satisfactory reason to resolve on the spot, or open a new ticket for that work.

That was addressed by someone else: https://news.ycombinator.com/item?id=45705759 . Also that just sounds like comments with extra steps. What is the meaning of plain comments in these teams?

So, what do you mean by "addressed"? Because what you are describing is not a third state, but passive aggressive block.

You want to block me, but also want it to look like my decision instead of yours.

> So, what do you mean by "addressed"?

Doing "something", that solves the issue that is described in the comment?

That can range from actually changing the code, putting a comment in, over updating the documentation, moving the code somewhere else, to merging the PR unchanged and opening a new PR, to not doing anything.

> You want to block me, but also want it to look like my decision instead of yours.

Because it IS your decision. What the comment means for the effect on your PR depends on how you address it. "changing the code" -> "new code review"; "putting a comment in", "updating the documentation" -> don't need approval, but expect questions; "not doing anything" -> get your approval from someone else, you won't get it from me, but if someone else is fine with it, maybe I am wrong.

I DO NOT want to block you, that's why I want to be able to only leave a comment.

Examples:

"This conflicts with POSIX and SUSv4 ..." -> (but the old code also did) "document that diversion and leave a point in todo.txt"

"This is highly surprising for users" -> "add a warning or a confirmation"

"This breaks the workflow for X people (including me), but this was undocumented behaviour" -> "Consider whether the feature is worth it and we care about this"

"This conflicts with my understanding of RFC1234 in combination with 'random blog post from The Old New Thing' and documentation in 'different vendor's codebase'" -> "Yes, I know, I did it on purpose, because of footnote in RFC5678 and random comment on HN proving why Raymond Chen is wrong; will document that".

The answer to all these comments could also be "Oops, didn't thought of that, now I need to change the code / throw everything away.", but this entirely depends on your state of mind, which I don't have introspection to. If I would start researching that problem in depth, then I could also do the PR myself, that is work, that you already did. I only want to make sure, that you are aware. If you are, consider it approved, if you are not, consider it blocked.

> call or something.

I kinda hate this.

It will feel efficient for the two people involved, but I'd actually want that exchange to be public and in writing, as we're talking about code that will go to production and will be read, debugged and maintained by dozen of other people from there.

Especially if it was complex enough to be discussed to these lengths. I don't care about code comments and don't trust people to properly maintain them, as long as MRs have the original context and discussion of the decisions. Shorting these discussions is a net loss to the org IMHO.

No one will read that discussion, ever. And it is such an epic waste of time to write down all these nitpicky discussions.

And a call where we react to each other takes 3 minutes max while the exchange through comments can take a week. Or, 2 days but both of you need to interrupt what you are doing a lot to answer quickly.

It is just absurd to treat code review as a discussion forum.

I spend half of my life on a legacy codebase, and the past PRs are my go to to understand anything mildly tricky in there.

My biggest realization was that the code comments were meaningless and borderline deceptive compared to the actual discussions and the target specs in the MRs. There's such a gap between what people want to explain or think will be useful, and the info actually needed. And I don't blame them, I'm not sure I'd do better.

My favorite is threads like "This name makes no sense" -> "Sure, give me a better name for X doing Y to Z" -> "Nevermind", which completely explains some of the insane naming, what they actually meant by it, and would never be left in any half official documentation.

I generally agree with you, but this:

"This name makes no sense" -> "Sure, give me a better name for X doing Y to Z" -> "Nevermind"

Should really result in a comment "/* X doing Y to Z */" before the declaration. I try to address these issues by treating PRs the same as phone calls, they can be gone tomorrow, so I put all the things in the code or the commit message. But of course I am not perfect, the people after me will determine whether I was successful.

You can get the best of both worlds by doing a phone call and then writing your insights in the commit message.