Live data from Hacker News

Ask HN: I'm tired of intense code review cycles

news.ycombinator.com

11–20 of 55 posts

Re: Ask HN: I'm tired of intense code review cycles

#11
There are different levels of suggestions, and if your team don't have them you could work on introducing them.

On my current job, one of guidelines says that if something is not important/very personal preference, you can prefix the comment with words "nit:" that means "nitpicking", and author can either change if it sees fit, or just acknowledge comment and go on.

Other thing is not to be shy to tell that suggested change is outside of the scope of this changeset, and will/may/might be addressed later.

Sometimes, if there is a lot of back and forth, it is easier to set pair programming session and address everything in one session.

Last option is raise that with management, if you can justify your opinion that such loop doesn't bring the value for the time spent on it, especially if you know that there are other engineers in the team who feel the same.

Re: Ask HN: I'm tired of intense code review cycles

#12

Some of this is a senior engineer justifying his own job. One time I copied code almost verbatim from another highly regarded team. Senior engineer ripped it, claiming it wasn’t at standard for the company. Then I told him it was from team x. A lot of this is bravado and a dog and pony show. You need code quality, but the important things are at the software architecture level/abstractions.

That’s hilarious, what did the senior engineer say?

DRY

Re: Ask HN: I'm tired of intense code review cycles

#13
I believe a variant of: "It is a well-known fact that those people who must want to rule people are, ipso facto, those least suited to do it... anyone who is capable of getting themselves made President should on no account be allowed to do the job."

... applies to code reviewers.

I like Google's review standard:

"In general, reviewers should favor approving a [PR] once it is in a state where it definitely improves the overall code health of the system being worked on, even if the [PR] isn’t perfect."

https://google.github.io/eng-practices/review/reviewer/stand...

Re: Ask HN: I'm tired of intense code review cycles

#14

I believe a variant of: "It is a well-known fact that those people who must want to rule people are, ipso facto, those least suited to do it... anyone who is capable of getting themselves made President should on no account be allowed to do the job." ... applies to code reviewers. I like Google's review standard: "In general, reviewers should favor approving a [PR] once it is in a state where it definitely improves t…

I often approve a PR with the comment "Approved with comments".

In other words, I didn't notice any bugs but I have some suggestions. I'll let you decide if you want to make the changes or not.

Re: Ask HN: I'm tired of intense code review cycles

#16
>- The focus of reviews is not on correctness but stuff like naming, docstrings, and design.

I agree that focusing on naming/docstrings doesn't make much sense, but design feels like it's fair game. What type of "design" stuff is showing up in your reviews?

Re: Ask HN: I'm tired of intense code review cycles

#17
post #14

I believe a variant of: "It is a well-known fact that those people who must want to rule people are, ipso facto, those least suited to do it... anyone who is capable of getting themselves made President should on no account be allowed to do the job." ... applies to code reviewers. I like Google's review standard: "In general, reviewers should favor approving a [PR] once it is in a state where it definitely improves t…

I often approve a PR with the comment "Approved with comments". In other words, I didn't notice any bugs but I have some suggestions. I'll let you decide if you want to make the changes or not.

+1. The job of reviewer on shared codebases is advisory, not as a gatekeeper.

If people choose to ignore comments, then that's a behavioral issue worth escalating.

Re: Ask HN: I'm tired of intense code review cycles

#20
post #14

I believe a variant of: "It is a well-known fact that those people who must want to rule people are, ipso facto, those least suited to do it... anyone who is capable of getting themselves made President should on no account be allowed to do the job." ... applies to code reviewers. I like Google's review standard: "In general, reviewers should favor approving a [PR] once it is in a state where it definitely improves t…

I often approve a PR with the comment "Approved with comments". In other words, I didn't notice any bugs but I have some suggestions. I'll let you decide if you want to make the changes or not.

I've been in teams where those comments are never addressed because the pressure is on "push something that roughly solves the task so you can show something during demo".
Post reply on HN