[watwut]

It continues with "and need to be explained why each change is okay". If reviewer suspects there is a bug, reviewer should check for it instead of having reviewee explain him or defend every detail. It is micromanagement this way - not just being similar or kinda like micromanagement, but it is literally it. If every five lines big code change in run of the mill fronted requires two hours long negotiation, then something is wrong.

When it is unreadable and hard to see whether it works it is different thing, but then the complain in review should be some specific variant of "hard to read".

I review mostly for architectural compliance and "bad idioms" or code smells. There is difference between not like I would write it and badly written mess and many programmers confuse them.

[gizmo686]

>If reviewer suspects there is a bug, reviewer should check for it instead of having reviewee explain him or defend every detail.

It is not a matter of if the reviewer "suspects there is a bug", but rather a matter of if the reviewer is convinced that there is not a bug.

If the reviewer needs to have someone explain why every minor code change is correct, they are either lazy or incompetent (or the code is badly written). Further, it is generally a bad idea to go to the person who wrote the code to explain why it is correct, as you are then much more likely to make the same mistake the original author made. However, being "okay" often goes beyond correctness, and into business decisions. Often times, these business are not documented (or if they are documented, it is in some management document that is not linked to from the code), so in order to review it, the reviewer has to ask the author what the bussiness consideration that led to the change was.

[watwut]

And other times, reviewer is doing that to look detail oriented and responsible. He can do that so everybody sees how great and attentive he is. It is opposite of laziness in such case, but still unfair to reviewee.

It is similar with being petty in code review - sometimes people do it because they think it makes them look like better coders.

As for checking business case, yes sometimes you have to do it. But there is also such a thing as doing it too often and second guessing every requirement that you see during code review. This has tricle down effect on analysts/managers/or even customers (he got pissed after a while of this - true story) who then have to litigate and defend every detail and solve programmers conflicts over insignificant details.

[jt2190]

> However, being "okay" often goes beyond correctness, and into business decisions.

Code reviews occur far too late in the development process for an uninvolved developer to provide a good, timely review of “bigger picture” concerns like business consideration (or software design) simply because the reviewing developer needs time to come up to speed on why decisions were made. In these cases it’s better to have the reviewing developer involved in a review of business requirements before the code is written. The code review can then stay focused on code.

[watwut]

Imo, business reasonability should be reviewed after the coder finished work, but not so much by programmer. It should be done by analyst, project manager or tester who knows a lot about the business. Basically, it should be done by person who communicated with customer or is responsible for overall vision.

[menor]

> I review mostly for architectural compliance and "bad idioms" or code smells. There is difference between not like I would write it and badly written mess and many programmers confuse them.

This ^ I just had this discussion last week in a PR, if the comment is a matter of opinion, the whole team should agree and put a rule in the linter or style guide. If not every one is free to have their own preferences.