There's a massive difference in communicated intent between overriding a blocking review and not blocking at all. Not blocking is a signal that you have no objection to merging the change as is.
If you care about whether your concerns are addressed by further discussion, block. If you don't actually care about whether something happens as indicated by not blocking, it's passive aggressive to expect more work.
People forget that you can just as easily block pending responses to comments and then approve after.
Sure, ok, so then they're addressed by the decision to ignore them because you don't care enough about them to make them blocking and there are other things to work on.
I think your understanding of "ignore them" is different than mine. When I say ignore them I mean ignore them, not do more labor to document a thought process in response to them which is the opposite of ignoring them.
If you refuse to do something on a problem I raise, one of us should be fired. Either you, because you refuse to work for what you get money, or me because I hand out advice on random stuff I am not supposed to do.
If you did ask me for my opinion, but ignore them, that's a refusal to work on your side. If you didn't asked me and I still started to mess with your work, then either I did found a serious issue and you should be grateful, if you ignore that, that is misconduct, or I spread nonsense around and should get a meeting with the employer on why I harass my colleagues.
Creating documentation is the professional way of ignoring problems. If I don't do that, I am acting against the profession I am employed as. Sweeping evidence of problems under the rack, will be later classified as something from at best misconduct to at worst malicious and criminal.
> If you refuse to do something on a problem I raise
If you think there's a problem and you don't block, then you're the one being negligent. If you review without blocking, you're indicating that you see no reasons why the code shouldn't be merged as is.
> If you did ask me for my opinion, but ignore them
In this scenario I saw that you don't see anything that needs to change. That is a direct expression of opinion from you by reviewing and not blocking. I am following your input efficiently by disregarding optional side quests you want to invoke.
The problem in this scenario is that you want to say "I do not think you need to do anything else", which is what it means to review and not block, while simultaneously expecting the person to do something else. Your contradictory input is what creates the problem here.
> you want to say "I do not think you need to do anything else", which is what it means to review and not block
> you're indicating that you see no reasons why the code shouldn't be merged as is
> In this scenario I saw that you don't see anything that needs to change.
This is a definition you introduced and I started the thread with saying that I disagree with this meaning. "I do not see anything that needs to change/you need to do" is indicated by me accepting the PR or just not leaving a comment at all. Me saying "you should consider fixing that/ bringing your code in concordance with Spec XY", is certainly not a case of that.
> If you think there's a problem and you don't block, then you're the one being negligent.
If I know for sure, that this is a real problem I would do that, but it's still you who choose to do a breaking change, after having known of a problem.
> optional side quests you want to invoke
If someone would want to put optional side quests in my PR, I would tell them to update the specification or create their own PR, but leave my PR alone.
You essentially say that you will only fix issues, when your colleagues force you to address them, by preventing you from merging code changes unless you fix them, and not just because you know your code is buggy. This doesn't sound like a healthy work culture to me. It should be in your interest to make your code non-buggy, not for your colleagues to need to force you to do that. Also this doesn't scale at all. Do you do the same to your colleagues? Then this seems like a weird workflow. Or do you expect this to be a one-way street?
So essentially I understand this be like this dialog:
A: Hi, I implemented that new feature ... Can you have a look.
B: Looks mostly fine... Are you sure line 18 is not a bug, because ...
A: No it isn't./ Oops, it is.
[If there is enough time A can explain this, or just do the fix and talk about it during lunch break.]
B: Ok, let's merge this.
I understand your dialog to go:
A: Hi, I implemented that new feature ... Can you have a look.
B: Looks mostly fine... Are you sure line 18 is not a bug, because ...
A: Force me to deal with it, otherwise I will just merge.
You obviously don't care about that problem, otherwise you would have told me, that the RFC I read yesterday is superseded,
because you can read my mind and know which RFC I had in mind when I wrote that.
B: ?????
A PR is a formalization of the above dialog, because most people are lazy and don't have that dialog on their own. In a PR, A doesn't need to answer whether A considers that to be a bug, because that's normally implicit in whether A has fixed that or not. With A from the second dialog this obviously doesn't work, but I think in this case no workflow will bring A to care about the code having no bugs, and this is behaviour I would describe as something from misconduct to malicious.
If you believe that line 18 may be a bug, not blocking is your negligence. It is your duty as the reviewer to block pending resolution of things that you believe may be bugs. Since we're talking about jobs, that is your job, not theirs. Otherwise you are saying "I believe you wrote code that may cause problems, but I'm not willing to do anything about it", which is being a lazy reviewer.
