[HN Gopher] Code reviews: A success story
___________________________________________________________________
Code reviews: A success story
Author : mu0n
Score : 24 points
Date : 2025-01-13 11:02 UTC (2 days ago)
(HTM) web link (blogsystem5.substack.com)
(TXT) w3m dump (blogsystem5.substack.com)
| moshegramovsky wrote:
| AI is pretty good at code reviews. For reference I use chatgpt
| and Gemini. It's very helpful.
|
| An AI tool that could convert large scale changes into a set of
| small commits would be amazing.
| jasonpeacock wrote:
| While I am a proponent of code reviews, what this article
| actually described is mentoring of a junior engineer by a senior
| engineer, and requiring effective testing to be implemented
| alongside the feature.
|
| It also shows a broken culture where the other reviewers were not
| engaged and committed to the success of the feature. When a bug
| escapes, it's both the author _and_ reviewer at fault.
| strongpigeon wrote:
| Only somewhat related, but I'd pay decent money to have access to
| the whole Piper/CitC/Critique/Code Search stack. As much as I've
| tried to like it, I just don't really like Github's code review
| tool.
| tristanb wrote:
| Shameless plug, but we built http://CodePeer.com - to bring
| Critique like features to everyone else. Take it for a spin if
| you like!
| mtlynch wrote:
| Former Googler. I also miss Critique/Gerrit. I've tried a bunch
| of alternatives, and I like CodeApprove:
|
| https://codeapprove.com/
|
| It's great if you have a team that does code reviews. It works
| less well for reviewing contributions from external
| contributors on an open-source project,a as the contributor
| likely just wants to get their PR merged and doesn't want to
| learn a special reviewing tool.
|
| No affiliation, just a happy customer.
| awkward wrote:
| Github's code review tool is uniquely bad. Notably it presents
| every comment as blocking and requiring sign off - even a "Glad
| someone cleaned this up! <thumbs up emoji>" needs clearing
| before merge.
|
| It also has some UX snafus that cause reviewers to write a
| number of comments and then forget to publish any of them,
| leading to a lot of interactions along the lines of "I thought
| you were going to review my PR?" "I already did?"
| spankalee wrote:
| I stopped reading after that opening paragraph. I don't know of
| anyone I take seriously who thinks that code reviews are bad
| practice or pure red tape.
| hnlurker22 wrote:
| Thankfully I'm not working with you
| hamandcheese wrote:
| Mandatory code review definitely creates red tape. Every place
| I've been with mandatory code review, I always see people
| "looking for a stamp".
|
| At my current job, code review requirements are set on a per-
| folder basis, with many folders not requiring a review. People
| ask for a review because they want a review (or, sometimes,
| they dont. For example, I don't ask someone to review every
| quick one-liner of the systems I am an expert in).
| rapfaria wrote:
| Same.
|
| _> Code reviews have a bad rap: they are antagonistic in
| nature and, sometimes, pure red tape._
|
| I wonder if folks know that this is _a job_? What are you gonna
| do, not do it? Cry at night because you forgot for the hundreth
| time to add token strings to translation files and not be hard-
| coded? Come on.
| hnlurker22 wrote:
| A few days ago there was an article on HN on how engineers
| abuse code reviews. It's just a tool, the outcome is
| different based on who's reviewing. If you think code review
| is intrinsically good then I'm glad I'm not working with you
| either
| hnlurker22 wrote:
| The author describes how _his_ code reviews that _he_ gave others
| are successful from _his_ own point of view.
| awkward wrote:
| Don't worry, he also asked his own reviewee, who said the
| reviews were helpful and in no way obnoxious.
| pavel_lishin wrote:
| But he does back it up with actual facts (as far as we can
| trust the author to tell the truth) - the feature that the
| author gave feedback on shipped without any issues. (The
| article actually doesn't say whether A was fixed-and-bug-free
| before B shipped, but it certainly sounds like B was less
| stressful to ship.)
| frankfrank13 wrote:
| > I also pushed for breaking large changes into smaller commits,
| because it is impossible to do a thorough review otherwise.
|
| I've found this to be key for both giving and receiving feedback.
| Even small _breaking_ commits are better in a lot of cases
| because they invite feedback in a way that 1000+ lines don 't.
___________________________________________________________________
(page generated 2025-01-15 23:00 UTC)